Fix column offsets for source mapped code (#197)

Author: calebeby

.changeset/seven-fireants-shop.md

@@ -0,0 +1,5 @@
+---
+'pleasantest': patch
+---
+
+Fix column offsets when esbuild is disabled

src/index.ts

@@ -371,17 +371,18 @@ const createTab = async ({
 
       const { SourceMapConsumer } = await import('source-map');
       const consumer = await new SourceMapConsumer(map as any);
-      const sourceLocation = consumer.originalPositionFor({ line, column });
+      const sourceLocation = consumer.originalPositionFor({
+        line,
+        column: column - 1, // Source-map uses zero-based column numbers
+      });
       consumer.destroy();
-      if (sourceLocation.line === null || sourceLocation.column === null)
-        return stackItem.raw;
-      const mappedColumn = sourceLocation.column + 1;
-      const mappedLine = sourceLocation.line;
-      const mappedPath = sourceLocation.source || url.pathname;
       return printStackLine(
-        mappedPath,
-        mappedLine,
-        mappedColumn,
+        join(process.cwd(), url.pathname),
+        sourceLocation.line ?? line,
+        sourceLocation.column === null
+          ? column
+          : // Convert back from zero-based column to 1-based
+            sourceLocation.column + 1,
         stackItem.name,
       );
     });

src/module-server/index.ts

@@ -65,7 +65,7 @@ export const createModuleServer = async ({
   const requestCache = new Map<string, SourceDescription>();
   const middleware: polka.Middleware[] = [
     indexHTMLMiddleware,
-    jsMiddleware({ root, plugins, requestCache }),
+    await jsMiddleware({ root, plugins, requestCache }),
     cssMiddleware({ root }),
     staticMiddleware({ root }),
   ];

src/module-server/middleware/js.ts

@@ -29,11 +29,11 @@ const getResolveCacheKey = (spec: string, from: string) =>
 
 // Minimal version of https://github.com/preactjs/wmr/blob/main/packages/wmr/src/wmr-middleware.js
 
-export const jsMiddleware = ({
+export const jsMiddleware = async ({
   root,
   plugins,
   requestCache,
-}: JSMiddlewareOpts): polka.Middleware => {
+}: JSMiddlewareOpts): Promise<polka.Middleware> => {
   interface ResolveCacheEntry {
     buildId: number;
     resolved: PartialResolvedId;
@@ -58,7 +58,8 @@ export const jsMiddleware = ({
 
   const rollupPlugins = createPluginContainer(plugins);
 
-  rollupPlugins.buildStart();
+  await rollupPlugins.options();
+  await rollupPlugins.buildStart();
 
   return async (req, res, next) => {
     const buildId =

src/module-server/plugins/esbuild-plugin.ts

@@ -24,6 +24,7 @@ export const esbuildPlugin = (
         ...esbuildOptions,
       })
       .catch((error) => {
+        if (!('errors' in error)) throw error;
         const err = error.errors[0];
         this.error(err.text, {
           line: err.location.line,

src/module-server/rollup-plugin-container.ts

@@ -36,6 +36,7 @@
   - relative resolution fixed to handle '.' for ctx.resolveId
   - Source map handling added to transform hook
   - Error handling (with code frame, using source maps) added to transform hook
+  - Stubbed out options hook was added
   */
 
 import { resolve, dirname } from 'path';
@@ -212,11 +213,9 @@ export const createPluginContainer = (plugins: Plugin[]) => {
     ) {
       let code = originalCode;
       // TODO: if any of the transforms is missing sourcemaps, then there should be no source maps emitted
-      const sourceMaps: (DecodedSourceMap | RawSourceMap)[] = [];
-      if (inputMap)
-        sourceMaps.push(
-          typeof inputMap === 'string' ? JSON.parse(inputMap) : inputMap,
-        );
+      const sourceMaps: (DecodedSourceMap | RawSourceMap)[] = inputMap
+        ? [typeof inputMap === 'string' ? JSON.parse(inputMap) : inputMap]
+        : [];
       for (plugin of plugins) {
         if (!plugin.transform) continue;
         try {
@@ -273,6 +272,14 @@ export const createPluginContainer = (plugins: Plugin[]) => {
       };
     },
 
+    async options() {
+      for (plugin of plugins) {
+        // Since we don't have "input options", we just pass {}
+        // This hook must be called for @rollup/plugin-babel
+        await plugin.options?.call(ctx as any, {});
+      }
+    },
+
     async load(id: string): Promise<LoadResult> {
       for (plugin of plugins) {
         if (!plugin.load) continue;

tests/utils/runJS.test.tsx

@@ -3,6 +3,7 @@ import type { PleasantestContext, PleasantestUtils } from 'pleasantest';
 import { printErrorFrames } from '../test-utils';
 import vuePlugin from 'rollup-plugin-vue';
 import aliasPlugin from '@rollup/plugin-alias';
+import babel from '@rollup/plugin-babel';
 import ansiRegex from 'ansi-regex';
 
 const createHeading = async ({
@@ -192,6 +193,23 @@ test(
   ),
 );
 
+test(
+  'Line/column offsets for source-mapped runtime error is correct even with esbuild disabled',
+  withBrowser({ moduleServer: { esbuild: false } }, async ({ utils }) => {
+    const error = await utils
+      .runJS('console.log(nothing)')
+      .catch((error) => error);
+    expect(await printErrorFrames(error)).toMatchInlineSnapshot(`
+      "ReferenceError: nothing is not defined
+      -------------------------------------------------------
+      tests/utils/runJS.test.tsx
+
+            .runJS('console.log(nothing)')
+                                ^"
+      `);
+  }),
+);
+
 test(
   'allows importing .tsx file, and errors from imported file are source mapped',
   withBrowser(async ({ utils, page }) => {
@@ -467,3 +485,37 @@ test(
     },
   ),
 );
+
+test(
+  '@rollup/plugin-babel works',
+  withBrowser(
+    {
+      moduleServer: {
+        esbuild: false,
+        plugins: [
+          babel({
+            extensions: ['.js', '.ts', '.tsx', '.mjs'],
+            babelHelpers: 'bundled',
+            presets: ['@babel/preset-typescript'],
+          }),
+        ],
+      },
+    },
+    async ({ utils }) => {
+      await utils.runJS("const foo: string = 'hello'");
+
+      // Check that source map from babel works correctly
+      const error = await utils
+        .runJS('console.log(nothing)')
+        .catch((error) => error);
+      expect(await printErrorFrames(error)).toMatchInlineSnapshot(`
+        "ReferenceError: nothing is not defined
+        -------------------------------------------------------
+        tests/utils/runJS.test.tsx
+
+              .runJS('console.log(nothing)')
+                                  ^"
+      `);
+    },
+  ),
+);