(fix) better css error diagnostics (#1009)

- Place error at start of script if coming from another file (#976)
- Add preprocessing wrapping for new TranspiledSvelteDocument for better error messages

Author: dummdidumm

packages/language-server/src/plugins/svelte/SvelteDocument.ts

@@ -18,7 +18,7 @@ import {
     TagInformation
 } from '../../lib/documents';
 import { SvelteConfig } from '../../lib/documents/configLoader';
-import { isNotNullOrUndefined } from '../../utils';
+import { getLastPartOfPath, isNotNullOrUndefined } from '../../utils';
 
 export type SvelteCompileResult = ReturnType<typeof compile>;
 
@@ -125,9 +125,13 @@ export class TranspiledSvelteDocument implements ITranspiledSvelteDocument {
 
         const filename = document.getFilePath() || '';
         const svelte = importSvelte(filename);
-        const preprocessed = await svelte.preprocess(document.getText(), config?.preprocess || [], {
-            filename
-        });
+        const preprocessed = await svelte.preprocess(
+            document.getText(),
+            wrapPreprocessors(config?.preprocess),
+            {
+                filename
+            }
+        );
 
         if (preprocessed.code === document.getText()) {
             return new TranspiledSvelteDocument(document.getText());
@@ -141,7 +145,7 @@ export class TranspiledSvelteDocument implements ITranspiledSvelteDocument {
                       // The "sources" array only contains the Svelte filename, not its path.
                       // For getting generated positions, the sourcemap consumer wants an exact match
                       // of the source filepath. Therefore only pass in the filename here.
-                      filename.replace(/\\/g, '/').split('/').pop() || ''
+                      getLastPartOfPath(filename)
                   )
                 : undefined
         );
@@ -394,6 +398,40 @@ export class SvelteFragmentMapper implements PositionMapper {
     }
 }
 
+/**
+ * Wrap preprocessors and rethrow on errors with more info on where the error came from.
+ */
+function wrapPreprocessors(preprocessors: PreprocessorGroup | PreprocessorGroup[] = []) {
+    preprocessors = Array.isArray(preprocessors) ? preprocessors : [preprocessors];
+    return preprocessors.map((preprocessor) => {
+        const wrappedPreprocessor: PreprocessorGroup = { markup: preprocessor.markup };
+
+        if (preprocessor.script) {
+            wrappedPreprocessor.script = async (args: any) => {
+                try {
+                    return await preprocessor.script!(args);
+                } catch (e) {
+                    e.__source = TranspileErrorSource.Script;
+                    throw e;
+                }
+            };
+        }
+
+        if (preprocessor.style) {
+            wrappedPreprocessor.style = async (args: any) => {
+                try {
+                    return await preprocessor.style!(args);
+                } catch (e) {
+                    e.__source = TranspileErrorSource.Style;
+                    throw e;
+                }
+            };
+        }
+
+        return wrappedPreprocessor;
+    });
+}
+
 async function transpile(
     document: Document,
     preprocessors: PreprocessorGroup | PreprocessorGroup[] = []

packages/language-server/src/plugins/svelte/features/getDiagnostics.ts

@@ -3,6 +3,7 @@ import { Diagnostic, DiagnosticSeverity, Position, Range } from 'vscode-language
 import { Document, isInTag, mapObjWithRangeToOriginal } from '../../../lib/documents';
 import { Logger } from '../../../logger';
 import { CompilerWarningsSettings } from '../../../ls-config';
+import { getLastPartOfPath } from '../../../utils';
 import { SvelteDocument, TranspileErrorSource } from '../SvelteDocument';
 
 /**
@@ -140,6 +141,12 @@ function getConfigLoadErrorDiagnostics(error: any): Diagnostic[] {
  * Try to infer a nice diagnostic error message from the transpilation error.
  */
 function getStyleErrorDiagnostics(error: any, document: Document): Diagnostic[] {
+    // Error could be from another file that was mixed into the Svelte file as part of preprocessing.
+    // Some preprocessors set the file property from which we can infer that
+    const isErrorFromOtherFile =
+        typeof error?.file === 'string' &&
+        getLastPartOfPath(error.file) !== getLastPartOfPath(document.getFilePath() || '');
+
     return [
         {
             message: getStyleErrorMessage(),
@@ -155,20 +162,22 @@ function getStyleErrorDiagnostics(error: any, document: Document): Diagnostic[]
             return getErrorMessage(error.message, 'style', hint);
         }
 
-        return (
+        const msg =
             error.formatted /* sass error messages have this */ ||
             error.message ||
-            'Style error. Transpilation failed.'
-        );
+            'Style error. Transpilation failed.';
+        return isErrorFromOtherFile ? 'Error in referenced file\n\n' + msg : msg;
     }
 
     function getStyleErrorRange() {
         const lineOffset = document.styleInfo?.startPos.line || 0;
         const position =
-            typeof error?.column === 'number' && typeof error?.line === 'number'
-                ? // Some preprocessors like sass or less return error objects with these attributes.
-                  // Use it to display a nice error message.
-                  Position.create(lineOffset + error.line - 1, error.column)
+            !isErrorFromOtherFile &&
+            // Some preprocessors like sass or less return error objects with these attributes.
+            // Use it to display message at better position.
+            typeof error?.column === 'number' &&
+            typeof error?.line === 'number'
+                ? Position.create(lineOffset + error.line - 1, error.column)
                 : document.styleInfo?.startPos || Position.create(0, 0);
         return Range.create(position, position);
     }

packages/language-server/src/utils.ts

@@ -26,6 +26,14 @@ export function normalizeUri(uri: string): string {
     return URI.parse(uri).toString();
 }
 
+/**
+ * Given a path like foo/bar or foo/bar.svelte , returns its last path
+ * (bar or bar.svelte in this example).
+ */
+export function getLastPartOfPath(path: string): string {
+    return path.replace(/\\/g, '/').split('/').pop() || '';
+}
+
 export function flatten<T>(arr: T[][]): T[] {
     return arr.reduce((all, item) => [...all, ...item], []);
 }