Add test to catch incorrect line numbers

EMH333 · Dec 2, 2022 · 3617764 · 3617764
1 parent 59eac93
commit 3617764
Show file tree

Hide file tree

Showing 4 changed files with 102 additions and 35 deletions.
diff --git a/index.ts b/index.ts
@@ -6,7 +6,7 @@ import { readFile, statSync } from "fs";
 
 import type { CompileOptions, Warning } from "svelte/types/compiler/interfaces";
 import type { PreprocessorGroup } from "svelte/types/compiler/preprocess/types";
-import type { OnLoadResult, Plugin, PluginBuild } from "esbuild";
+import type { OnLoadResult, Plugin, PluginBuild, Location } from "esbuild";
 
 interface esbuildSvelteOptions {
     /**
@@ -52,17 +52,21 @@ interface CacheData {
     dependencies: Map<string, Date>;
 }
 
-const convertMessage = ({ message, start, end, filename, frame }: Warning) => ({
-    text: message,
-    location: start &&
-        end && {
+function convertMessage({ message, start, end }: Warning, filename: string, source: string) {
+    let location: Partial<Location> | undefined;
+    if (start && end) {
+        let lineText = source.split(/\r\n|\r|\n/g)[start.line - 1];
+        let lineEnd = start.line === end.line ? end.column : lineText.length;
+        location = {
             file: filename,
             line: start.line,
             column: start.column,
-            length: start.line === end.line ? end.column - start.column : 0,
-            lineText: frame,
-        },
-});
+            length: lineEnd - start.column,
+            lineText,
+        };
+    }
+    return { text: message, location };
+}
 
 const shouldCache = (build: PluginBuild) =>
     build.initialOptions.incremental || build.initialOptions.watch;
@@ -261,7 +265,7 @@ export default function sveltePlugin(options?: esbuildSvelteOptions): Plugin {
 
                     const result: OnLoadResult = {
                         contents,
-                        warnings: warnings.map(convertMessage),
+                        warnings: warnings.map((e) => convertMessage(e, args.path, source)),
                     };
 
                     // if we are told to cache, then cache
@@ -280,7 +284,10 @@ export default function sveltePlugin(options?: esbuildSvelteOptions): Plugin {
 
                     return result;
                 } catch (e: any) {
-                    return { errors: [convertMessage(e)], watchFiles: previousWatchFiles };
+                    return {
+                        errors: [convertMessage(e, args.path, originalSource)],
+                        watchFiles: previousWatchFiles,
+                    };
                 }
             });
 

diff --git a/test/fixtures/preprocessing-sourcemaps/pp-sourcemaps.svelte b/test/fixtures/preprocessing-sourcemaps/pp-sourcemaps.svelte
@@ -1,33 +1,35 @@
 <script lang="typescript">
-    interface Test {
-        yellThisThing: String;
-    }
+  interface Test {
+    yellThisThing: String;
+  }
 
-    function event() {
-        tryThisThing({ yellThisThing: "something!" });
-        change();
-    }
+  function event() {
+    tryThisThing({ yellThisThing: "something!" });
+    change();
+  }
 
-    function tryThisThing(input: Test) {
-        console.log(input.yellThisThing.toUpperCase());
-    }
+  function tryThisThing(input: Test) {
+    console.log(input.yellThisThing.toUpperCase());
+  }
 
-    let inputBinding: HTMLInputElement;
+  let inputBinding: HTMLInputElement;
 
-    function change() {
-        inputBinding.value = "testing" + Math.round(Math.random() * 100);
-    }
+  function change() {
+    inputBinding.value = "testing" + Math.round(Math.random() * 100);
+  }
 </script>
 
-<style lang="scss">
-    @use "sourcemapImport.scss";
-    @use "sass:color";
-    .sourcemap {
-        $primary-color: #6b717f;
-        color: $primary-color;
-        border: 1px solid color.scale($primary-color, $lightness: 20%);
-    }
-</style>
-
 <button on:click={event} class="sourcemap">Yell "something"</button>
 <input type="text" name="testingInput" bind:this={inputBinding} />
+
+<img src="foo.jpg" />
+
+<style lang="scss">
+  @use "sourcemapImport.scss";
+  @use "sass:color";
+  .sourcemap {
+    $primary-color: #6b717f;
+    color: $primary-color;
+    border: 1px solid color.scale($primary-color, $lightness: 20%);
+  }
+</style>
diff --git a/test/sourcemapsTest.mjs b/test/sourcemapsTest.mjs
@@ -18,8 +18,12 @@ test("Preprocessor Sourcemap test", async () => {
         sourcemap: "external",
         outdir: "out",
         plugins: [sveltePlugin({ preprocess: [{ style: sass() }, typescript()] })],
+        logLevel: "silent",
     });
 
+    assert.equal(result.warnings.length, 1, "Should one warning"); // because of warnings tests
+    assert.equal(result.errors.length, 0, "Should not have errors");
+
     assert.equal(result.outputFiles.length, 4);
 
     for (let out of result.outputFiles) {

diff --git a/test/warnings.mjs b/test/warnings.mjs
@@ -2,6 +2,7 @@ import { test } from "uvu";
 import * as assert from "uvu/assert";
 import { build as _build } from "esbuild";
 import { typescript } from "svelte-preprocess-esbuild";
+import { sass } from "svelte-preprocess-sass";
 import sveltePlugin from "../dist/index.mjs";
 
 test("Can filter out warnings", async () => {
@@ -57,4 +58,57 @@ test("Can filter out warnings", async () => {
     assert.equal(resultsWithFilter.errors.length, 0, "Should not have errors");
 });
 
+test("Warnings are in the right spot", async () => {
+    const results = await _build({
+        entryPoints: ["./test/fixtures/warnings/entry.js"],
+        outdir: "../example/dist",
+        bundle: true,
+        write: false, //Don't write anywhere
+        sourcemap: true,
+        plugins: [
+            sveltePlugin({
+                preprocess: typescript(),
+                compilerOptions: { dev: true },
+            }),
+        ],
+        logLevel: "silent",
+    });
+
+    assert.equal(results.warnings.length, 2, "Should have two warnings");
+    assert.equal(results.errors.length, 0, "Should not have errors");
+    assert.equal(results.warnings[0].location.column, 7, "Column is correct");
+    assert.equal(results.warnings[0].location.line, 3, "Line is correct");
+    assert.equal(results.warnings[0].location.length, 9, "Length is correct");
+    assert.equal(
+        results.warnings[0].location.lineText,
+        "  {#if MY_GLOBAL}",
+        "Line text is correct"
+    );
+    assert.match(results.warnings[0].text, /'MY_GLOBAL' is not defined/);
+});
+
+test("Preprocessor errors are as expected", async () => {
+    const results = await _build({
+        entryPoints: ["./test/fixtures/preprocessing-sourcemaps/pp-sourcemaps.js"],
+        outdir: "../example/dist",
+        format: "esm",
+        minify: true,
+        bundle: true,
+        splitting: true,
+        write: false, //Don't write anywhere
+        sourcemap: "external",
+        outdir: "out",
+        plugins: [sveltePlugin({ preprocess: [{ style: sass() }, typescript()] })],
+        logLevel: "silent",
+    });
+
+    assert.equal(results.outputFiles.length, 4);
+    assert.equal(results.warnings.length, 1, "Should have one warnings");
+    assert.equal(results.errors.length, 0, "Should not have errors");
+
+    assert.equal(results.warnings[0].location.column, 0, "Column is correct");
+    assert.equal(results.warnings[0].location.line, 25, "Line is correct");
+    assert.equal(results.warnings[0].location.length, 21, "Length is correct");
+});
+
 test.run();