Fix bug where REPL inputs were compiled as ESM in ESM projects (#1959)

cspotcode · web-flow · commit 6b1fd40d91dd · 2023-02-13T22:57:28.000-05:00
* change repl virtual filename to .cts extension so it compiles as cjs even in ESM packages with module:nodenext

* fix test

* Fix on TS &lt; 4.5

* fix failing tests hopefully
Also split test/repl/helpers into multiple files
diff --git a/src/bin.ts b/src/bin.ts
@@ -517,7 +517,8 @@ function phase4(payload: BootstrapState) {
     module.paths = (Module as any)._nodeModulePaths(cwd);
   }
   if (executeRepl) {
-    const state = new EvalState(join(cwd, REPL_FILENAME));
+    // correct path is set later
+    const state = new EvalState('');
     replStuff = {
       state,
       repl: createRepl({
@@ -541,6 +542,10 @@ function phase4(payload: BootstrapState) {
     },
   });
   register(service);
+
+  if (replStuff)
+    replStuff.state.path = join(cwd, REPL_FILENAME(service.ts.version));
+
   if (isInChildProcess)
     (
       require('./child/child-loader') as typeof import('./child/child-loader')
diff --git a/src/file-extensions.ts b/src/file-extensions.ts
@@ -50,6 +50,10 @@ const nodeDoesNotUnderstand: readonly string[] = [
   '.mts',
 ];
 
+export function tsSupportsMtsCtsExts(tsVersion: string) {
+  return versionGteLt(tsVersion, '4.5.0');
+}
+
 /**
  * [MUST_UPDATE_FOR_NEW_FILE_EXTENSIONS]
  * @internal
@@ -60,7 +64,7 @@ export function getExtensions(
   tsVersion: string
 ) {
   // TS 4.5 is first version to understand .cts, .mts, .cjs, and .mjs extensions
-  const tsSupportsMtsCtsExts = versionGteLt(tsVersion, '4.5.0');
+  const supportMtsCtsExts = tsSupportsMtsCtsExts(tsVersion);
 
   const requiresHigherTypescriptVersion: string[] = [];
   if (!tsSupportsMtsCtsExts)
@@ -78,11 +82,11 @@ export function getExtensions(
   const compiledJsxUnsorted: string[] = [];
 
   if (config.options.jsx) compiledJsxUnsorted.push('.tsx');
-  if (tsSupportsMtsCtsExts) compiledJsUnsorted.push('.mts', '.cts');
+  if (supportMtsCtsExts) compiledJsUnsorted.push('.mts', '.cts');
   if (config.options.allowJs) {
     compiledJsUnsorted.push('.js');
     if (config.options.jsx) compiledJsxUnsorted.push('.jsx');
-    if (tsSupportsMtsCtsExts) compiledJsUnsorted.push('.mjs', '.cjs');
+    if (supportMtsCtsExts) compiledJsUnsorted.push('.mjs', '.cjs');
   }
 
   const compiledUnsorted = [...compiledJsUnsorted, ...compiledJsxUnsorted];
diff --git a/src/repl.ts b/src/repl.ts
@@ -15,6 +15,7 @@ import * as assert from 'assert';
 import type * as tty from 'tty';
 import type * as Module from 'module';
 import { builtinModules } from 'module';
+import { tsSupportsMtsCtsExts } from './file-extensions';
 
 // Lazy-loaded.
 let _processTopLevelAwait: (src: string) => string | null;
@@ -43,7 +44,9 @@ export const STDIN_FILENAME = `[stdin].ts`;
 /** @internal */
 export const STDIN_NAME = `[stdin]`;
 /** @internal */
-export const REPL_FILENAME = '<repl>.ts';
+export function REPL_FILENAME(tsVersion: string) {
+  return tsSupportsMtsCtsExts(tsVersion) ? '<repl>.cts' : '<repl>.ts';
+}
 /** @internal */
 export const REPL_NAME = '<repl>';
 
@@ -130,6 +133,11 @@ export interface CreateReplOptions {
   ignoreDiagnosticsThatAreAnnoyingInInteractiveRepl?: boolean;
 }
 
+interface StartReplInternalOptions extends ReplOptions {
+  code?: string;
+  forceToBeModule?: boolean;
+}
+
 /**
  * Create a ts-node REPL instance.
  *
@@ -148,13 +156,21 @@ export interface CreateReplOptions {
  */
 export function createRepl(options: CreateReplOptions = {}) {
   const { ignoreDiagnosticsThatAreAnnoyingInInteractiveRepl = true } = options;
-  let service = options.service;
   let nodeReplServer: REPLServer;
   // If `useGlobal` is not true, then REPL creates a context when started.
   // This stores a reference to it or to `global`, whichever is used, after REPL has started.
   let context: Context | undefined;
-  const state =
-    options.state ?? new EvalState(join(process.cwd(), REPL_FILENAME));
+  let state: EvalState;
+  let mustSetStatePath = false;
+  if (options.state) {
+    state = options.state;
+  } else {
+    // Correct path set later
+    state = new EvalState('');
+    mustSetStatePath = true;
+  }
+  let service: Service;
+  if (options.service) setService(options.service);
   const evalAwarePartialHost = createEvalAwarePartialHost(
     state,
     options.composeWithEvalAwarePartialHost
@@ -167,6 +183,8 @@ export function createRepl(options: CreateReplOptions = {}) {
       ? console
       : new Console(stdout, stderr);
 
+  const declaredExports = new Set();
+
   const replService: ReplService = {
     state: options.state ?? new EvalState(join(process.cwd(), EVAL_FILENAME)),
     setService,
@@ -186,6 +204,8 @@ export function createRepl(options: CreateReplOptions = {}) {
 
   function setService(_service: Service) {
     service = _service;
+    if (mustSetStatePath)
+      state.path = join(process.cwd(), REPL_FILENAME(service.ts.version));
     if (ignoreDiagnosticsThatAreAnnoyingInInteractiveRepl) {
       service.addDiagnosticFilter({
         appliesToAllFiles: false,
@@ -200,7 +220,19 @@ export function createRepl(options: CreateReplOptions = {}) {
     }
   }
 
+  // Hard fix for TypeScript forcing `Object.defineProperty(exports, ...)`.
+  function declareExports() {
+    if (declaredExports.has(context)) return;
+    runInContext(
+      'exports = typeof module === "undefined" ? {} : module.exports;',
+      state.path,
+      context
+    );
+    declaredExports.add(context);
+  }
+
   function evalCode(code: string) {
+    declareExports();
     const result = appendCompileAndEvalInput({
       service: service!,
       state,
@@ -218,6 +250,7 @@ export function createRepl(options: CreateReplOptions = {}) {
     context: Context;
   }) {
     const { code, enableTopLevelAwait, context } = options;
+    declareExports();
     return appendCompileAndEvalInput({
       service: service!,
       state,
@@ -321,9 +354,7 @@ export function createRepl(options: CreateReplOptions = {}) {
   }
 
   // Note: `code` argument is deprecated
-  function startInternal(
-    options?: ReplOptions & { code?: string; forceToBeModule?: boolean }
-  ) {
+  function startInternal(options?: StartReplInternalOptions) {
     const { code, forceToBeModule = true, ...optionsOverride } = options ?? {};
     // TODO assert that `service` is set; remove all `service!` non-null assertions
 
@@ -362,12 +393,11 @@ export function createRepl(options: CreateReplOptions = {}) {
 
     // Bookmark the point where we should reset the REPL state.
     const resetEval = appendToEvalState(state, '');
-
     function reset() {
       resetEval();
 
-      // Hard fix for TypeScript forcing `Object.defineProperty(exports, ...)`.
-      runInContext('exports = module.exports', state.path, context);
+      declareExports();
+
       if (forceToBeModule) {
         state.input += 'export {};void 0;\n';
       }
diff --git a/src/test/helpers/ctx-ts-node.ts b/src/test/helpers/ctx-ts-node.ts
@@ -39,6 +39,7 @@ export async function installTsNode() {
     while (true) {
       try {
         rimrafSync(join(TEST_DIR, '.yarn/cache/ts-node-file-*'));
+        writeFileSync(join(TEST_DIR, 'yarn.lock'), '');
         const result = await promisify(childProcessExec)(
           `yarn --no-immutable`,
           {
diff --git a/src/test/repl/helpers/ctx-repl.ts b/src/test/repl/helpers/ctx-repl.ts
@@ -1,8 +1,9 @@
-import { PassThrough } from 'stream';
-import { delay, TEST_DIR, tsNodeTypes, ctxTsNode } from '../helpers';
-import type { ExecutionContext } from 'ava';
-import { test, expect } from '../testlib';
+import type { ExecutionContext } from '@cspotcode/ava-lib';
 import { expectStream } from '@cspotcode/expect-stream';
+import { PassThrough } from 'stream';
+import type { ctxTsNode } from '../../helpers/ctx-ts-node';
+import { delay, tsNodeTypes } from '../../helpers/misc';
+import { TEST_DIR } from '../../helpers/paths';
 
 export interface CreateReplViaApiOptions {
   registerHooks: boolean;
@@ -97,46 +98,3 @@ export async function ctxRepl(t: ctxTsNode.T) {
     };
   }
 }
-
-export const macroReplNoErrorsAndStdoutContains = test.macro(
-  (script: string, contains: string, options?: Partial<ExecuteInReplOptions>) =>
-    async (t: ctxRepl.T) => {
-      macroReplInternal(t, script, contains, undefined, contains, options);
-    }
-);
-export const macroReplStderrContains = test.macro(
-  (
-      script: string,
-      errorContains: string,
-      options?: Partial<ExecuteInReplOptions>
-    ) =>
-    async (t: ctxRepl.T) => {
-      macroReplInternal(
-        t,
-        script,
-        undefined,
-        errorContains,
-        errorContains,
-        options
-      );
-    }
-);
-
-async function macroReplInternal(
-  t: ctxRepl.T,
-  script: string,
-  stdoutContains: string | undefined,
-  stderrContains: string | undefined,
-  waitPattern: string,
-  options?: Partial<ExecuteInReplOptions>
-) {
-  const r = await t.context.executeInRepl(script, {
-    registerHooks: true,
-    startInternalOptions: { useGlobal: false },
-    waitPattern,
-    ...options,
-  });
-  if (stderrContains) expect(r.stderr).toContain(stderrContains);
-  else expect(r.stderr).toBe('');
-  if (stdoutContains) expect(r.stdout).toContain(stdoutContains);
-}
diff --git a/src/test/repl/helpers/macros.ts b/src/test/repl/helpers/macros.ts
@@ -0,0 +1,45 @@
+import type { ctxRepl, ExecuteInReplOptions } from './ctx-repl';
+import { expect, test } from '../../testlib';
+
+export const macroReplNoErrorsAndStdoutContains = test.macro(
+  (script: string, contains: string, options?: Partial<ExecuteInReplOptions>) =>
+    async (t: ctxRepl.T) => {
+      macroReplInternal(t, script, contains, undefined, contains, options);
+    }
+);
+export const macroReplStderrContains = test.macro(
+  (
+      script: string,
+      errorContains: string,
+      options?: Partial<ExecuteInReplOptions>
+    ) =>
+    async (t: ctxRepl.T) => {
+      macroReplInternal(
+        t,
+        script,
+        undefined,
+        errorContains,
+        errorContains,
+        options
+      );
+    }
+);
+
+async function macroReplInternal(
+  t: ctxRepl.T,
+  script: string,
+  stdoutContains: string | undefined,
+  stderrContains: string | undefined,
+  waitPattern: string,
+  options?: Partial<ExecuteInReplOptions>
+) {
+  const r = await t.context.executeInRepl(script, {
+    registerHooks: true,
+    startInternalOptions: { useGlobal: false },
+    waitPattern,
+    ...options,
+  });
+  if (stderrContains) expect(r.stderr).toContain(stderrContains);
+  else expect(r.stderr).toBe('');
+  if (stdoutContains) expect(r.stdout).toContain(stdoutContains);
+}
diff --git a/src/test/repl/helpers/misc.ts b/src/test/repl/helpers/misc.ts
@@ -0,0 +1,3 @@
+import { tsNodeTypes, tsSupportsMtsCtsExtensions } from '../../helpers';
+
+export const replFile = tsSupportsMtsCtsExtensions ? '<repl>.cts' : '<repl>.ts';
diff --git a/src/test/repl/repl-environment.spec.ts b/src/test/repl/repl-environment.spec.ts
@@ -14,7 +14,8 @@ import {
 import { dirname, join } from 'path';
 import { createExec, createExecTester } from '../exec-helpers';
 import { homedir } from 'os';
-import { ctxRepl } from './helpers';
+import { replFile } from './helpers/misc';
+import { ctxRepl } from './helpers/ctx-repl';
 
 const test = context(ctxTsNode).contextEach(ctxRepl);
 
@@ -100,7 +101,7 @@ test.suite(
               modulePath: typeof module !== 'undefined' && module.path,
               moduleFilename: typeof module !== 'undefined' && module.filename,
               modulePaths: typeof module !== 'undefined' && [...module.paths],
-              exportsTest: typeof exports !== 'undefined' ? module.exports === exports : null,
+              exportsTest: typeof exports !== 'undefined' && typeof module !== 'undefined' ? module.exports === exports : null,
               stackTest: new Error().stack!.split('\\n')[1],
               moduleAccessorsTest: eval('typeof fs') === 'undefined' ? null : eval('fs') === require('fs'),
               argv: [...process.argv]
@@ -197,7 +198,7 @@ test.suite(
           exportsTest: true,
           // Note: vanilla node uses different name. See #1360
           stackTest: expect.stringContaining(
-            `    at ${join(TEST_DIR, '<repl>.ts')}:4:`
+            `    at ${join(TEST_DIR, replFile)}:4:`
           ),
           moduleAccessorsTest: true,
           argv: [tsNodeExe],
@@ -350,7 +351,7 @@ test.suite(
           exportsTest: true,
           // Note: vanilla node uses different name. See #1360
           stackTest: expect.stringContaining(
-            `    at ${join(TEST_DIR, '<repl>.ts')}:4:`
+            `    at ${join(TEST_DIR, replFile)}:4:`
           ),
           moduleAccessorsTest: true,
           argv: [tsNodeExe],
@@ -428,7 +429,7 @@ test.suite(
 
             // Note: vanilla node uses different name. See #1360
             stackTest: expect.stringContaining(
-              `    at ${join(TEST_DIR, '<repl>.ts')}:1:`
+              `    at ${join(TEST_DIR, replFile)}:1:`
             ),
           },
         });
@@ -459,7 +460,7 @@ test.suite(
             exportsTest: true,
             // Note: vanilla node uses different name. See #1360
             stackTest: expect.stringContaining(
-              `    at ${join(TEST_DIR, '<repl>.ts')}:1:`
+              `    at ${join(TEST_DIR, replFile)}:1:`
             ),
             moduleAccessorsTest: true,
           },
diff --git a/src/test/repl/repl.spec.ts b/src/test/repl/repl.spec.ts
diff --git a/tests/repl-in-esm-package/package.json b/tests/repl-in-esm-package/package.json
diff --git a/tests/repl-in-esm-package/tsconfig.json b/tests/repl-in-esm-package/tsconfig.json

Original file line number	Diff line number	Diff line change
`@@ -39,6 +39,7 @@ export async function installTsNode() {`
`39`	`39`	`while (true) {`
`40`	`40`	`try {`
`41`	`41`	`rimrafSync(join(TEST_DIR, '.yarn/cache/ts-node-file-*'));`
	`42`	`+ writeFileSync(join(TEST_DIR, 'yarn.lock'), '');`
`42`	`43`	`const result = await promisify(childProcessExec)(`
`43`	`44`	`yarn --no-immutable`,
`44`	`45`	`{`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+import { tsNodeTypes, tsSupportsMtsCtsExtensions } from '../../helpers';`
	`2`	`+`
	`3`	`+export const replFile = tsSupportsMtsCtsExtensions ? '<repl>.cts' : '<repl>.ts';`