Two swc REPL fixes: combines #1541 and #1580 (#1602)

* failing swc transpiler repl test

* Fix

* Strip sourcemap comment from REPL JS before diffing and executing

* add comment about how repl does not do sourcemaps

* lint-fix

* fix

* fix

* sneaking in a fix for a windows test flake

Co-authored-by: Andy Stanberry <andystanberry@gmail.com>

src/index.ts

@@ -479,6 +479,8 @@ export interface Service {
   installSourceMapSupport(): void;
   /** @internal */
   enableExperimentalEsmLoaderInterop(): void;
+  /** @internal */
+  transpileOnly: boolean;
 }
 
 /**
@@ -1330,6 +1332,7 @@ export function create(rawOptions: CreateOptions = {}): Service {
     addDiagnosticFilter,
     installSourceMapSupport,
     enableExperimentalEsmLoaderInterop,
+    transpileOnly,
   };
 }
 

src/repl.ts

@@ -368,16 +368,20 @@ export function createRepl(options: CreateReplOptions = {}) {
       //   those starting with _
       //   those containing /
       //   those that already exist as globals
-      // Intentionally suppress type errors in case @types/node does not declare any of them.
-      state.input += `// @ts-ignore\n${builtinModules
-        .filter(
-          (name) =>
-            !name.startsWith('_') &&
-            !name.includes('/') &&
-            !['console', 'module', 'process'].includes(name)
-        )
-        .map((name) => `declare import ${name} = require('${name}')`)
-        .join(';')}\n`;
+      // Intentionally suppress type errors in case @types/node does not declare any of them, and because
+      // `declare import` is technically invalid syntax.
+      // Avoid this when in transpileOnly, because third-party transpilers may not handle `declare import`.
+      if (!service?.transpileOnly) {
+        state.input += `// @ts-ignore\n${builtinModules
+          .filter(
+            (name) =>
+              !name.startsWith('_') &&
+              !name.includes('/') &&
+              !['console', 'module', 'process'].includes(name)
+          )
+          .map((name) => `declare import ${name} = require('${name}')`)
+          .join(';')}\n`;
+      }
     }
 
     reset();
@@ -480,6 +484,8 @@ export function createEvalAwarePartialHost(
   return { readFile, fileExists };
 }
 
+const sourcemapCommentRe = /\/\/# ?sourceMappingURL=\S+[\s\r\n]*$/;
+
 type AppendCompileAndEvalInputResult =
   | { containsTopLevelAwait: true; valuePromise: Promise<any> }
   | { containsTopLevelAwait: false; value: any };
@@ -525,8 +531,23 @@ function appendCompileAndEvalInput(options: {
 
   output = adjustUseStrict(output);
 
+  // Note: REPL does not respect sourcemaps!
+  // To properly do that, we'd need to prefix the code we eval -- which comes
+  // from `diffLines` -- with newlines so that it's at the proper line numbers.
+  // Then we'd need to ensure each bit of eval-ed code, if there are multiples,
+  // has the sourcemap appended to it.
+  // We might also need to integrate with our sourcemap hooks' cache; I'm not sure.
+  const outputWithoutSourcemapComment = output.replace(sourcemapCommentRe, '');
+  const oldOutputWithoutSourcemapComment = state.output.replace(
+    sourcemapCommentRe,
+    ''
+  );
+
   // Use `diff` to check for new JavaScript to execute.
-  const changes = diffLines(state.output, output);
+  const changes = diffLines(
+    oldOutputWithoutSourcemapComment,
+    outputWithoutSourcemapComment
+  );
 
   if (isCompletion) {
     undo();

src/test/repl/repl.spec.ts

@@ -32,6 +32,16 @@ test('should run REPL when --interactive passed and stdin is not a TTY', async (
   expect(stdout).toBe('> 123\n' + 'undefined\n' + '> ');
 });
 
+test('should echo a value when using the swc transpiler', async () => {
+  const execPromise = exec(
+    `${CMD_TS_NODE_WITH_PROJECT_FLAG} --interactive  --transpiler ts-node/transpilers/swc-experimental`
+  );
+  execPromise.child.stdin!.end('400\n401\n');
+  const { err, stdout } = await execPromise;
+  expect(err).toBe(null);
+  expect(stdout).toBe('> 400\n> 401\n> ');
+});
+
 test('REPL has command to get type information', async () => {
   const execPromise = exec(`${CMD_TS_NODE_WITH_PROJECT_FLAG} --interactive`);
   execPromise.child.stdin!.end('\nconst a = 123\n.type a');
@@ -46,16 +56,20 @@ test('REPL has command to get type information', async () => {
 test.serial('REPL can be configured on `start`', async (t) => {
   const prompt = '#> ';
 
-  const { stdout, stderr } = await t.context.executeInRepl('const x = 3', {
-    registerHooks: true,
-    startInternalOptions: {
-      prompt,
-      ignoreUndefined: true,
-    },
-  });
+  const { stdout, stderr } = await t.context.executeInRepl(
+    `const x = 3\n'done'`,
+    {
+      waitPattern: "'done'",
+      registerHooks: true,
+      startInternalOptions: {
+        prompt,
+        ignoreUndefined: true,
+      },
+    }
+  );
 
   expect(stderr).toBe('');
-  expect(stdout).toBe(`${prompt}${prompt}`);
+  expect(stdout).toBe(`${prompt}${prompt}'done'\n`);
 });
 
 // Serial because it's timing-sensitive
@@ -455,29 +469,54 @@ test.suite('REPL works with traceResolution', (test) => {
   );
 });
 
-test.serial('REPL declares types for node built-ins within REPL', async (t) => {
-  const { stdout, stderr } = await t.context.executeInRepl(
-    `util.promisify(setTimeout)("should not be a string" as string)
-    type Duplex = stream.Duplex
-    const s = stream
-    'done'`,
-    {
-      registerHooks: true,
-      waitPattern: `done`,
-      startInternalOptions: {
-        useGlobal: false,
-      },
-    }
-  );
+test.suite('REPL declares types for node built-ins within REPL', (test) => {
+  test.runSerially();
+  test('enabled when typechecking', async (t) => {
+    const { stdout, stderr } = await t.context.executeInRepl(
+      `util.promisify(setTimeout)("should not be a string" as string)
+      type Duplex = stream.Duplex
+      const s = stream
+      'done'`,
+      {
+        registerHooks: true,
+        waitPattern: `done`,
+        startInternalOptions: {
+          useGlobal: false,
+        },
+      }
+    );
 
-  // Assert that we receive a typechecking error about improperly using
-  // `util.promisify` but *not* an error about the absence of `util`
-  expect(stderr).not.toMatch("Cannot find name 'util'");
-  expect(stderr).toMatch(
-    "Argument of type 'string' is not assignable to parameter of type 'number'"
-  );
-  // Assert that both types and values can be used without error
-  expect(stderr).not.toMatch("Cannot find namespace 'stream'");
-  expect(stderr).not.toMatch("Cannot find name 'stream'");
-  expect(stdout).toMatch(`done`);
+    // Assert that we receive a typechecking error about improperly using
+    // `util.promisify` but *not* an error about the absence of `util`
+    expect(stderr).not.toMatch("Cannot find name 'util'");
+    expect(stderr).toMatch(
+      "Argument of type 'string' is not assignable to parameter of type 'number'"
+    );
+    // Assert that both types and values can be used without error
+    expect(stderr).not.toMatch("Cannot find namespace 'stream'");
+    expect(stderr).not.toMatch("Cannot find name 'stream'");
+    expect(stdout).toMatch(`done`);
+  });
+
+  test('disabled in transpile-only mode, to avoid breaking third-party SWC transpiler which rejects `declare import` syntax', async (t) => {
+    const { stdout, stderr } = await t.context.executeInRepl(
+      `type Duplex = stream.Duplex
+      const s = stream
+      'done'`,
+      {
+        createServiceOpts: {
+          swc: true,
+        },
+        registerHooks: true,
+        waitPattern: `done`,
+        startInternalOptions: {
+          useGlobal: false,
+        },
+      }
+    );
+
+    // Assert that we do not get errors about `declare import` syntax from swc
+    expect(stdout).toBe("> undefined\n> undefined\n> 'done'\n");
+    expect(stderr).toBe('');
+  });
 });