Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/bin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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')
Expand Down
10 changes: 7 additions & 3 deletions src/file-extensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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];
Expand Down
50 changes: 40 additions & 10 deletions src/repl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>';

Expand Down Expand Up @@ -130,6 +133,11 @@ export interface CreateReplOptions {
ignoreDiagnosticsThatAreAnnoyingInInteractiveRepl?: boolean;
}

interface StartReplInternalOptions extends ReplOptions {
code?: string;
forceToBeModule?: boolean;
}

/**
* Create a ts-node REPL instance.
*
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -218,6 +250,7 @@ export function createRepl(options: CreateReplOptions = {}) {
context: Context;
}) {
const { code, enableTopLevelAwait, context } = options;
declareExports();
return appendCompileAndEvalInput({
service: service!,
state,
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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';
}
Expand Down
1 change: 1 addition & 0 deletions src/test/helpers/ctx-ts-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
{
Expand Down
52 changes: 5 additions & 47 deletions src/test/repl/helpers.ts → src/test/repl/helpers/ctx-repl.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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);
}
45 changes: 45 additions & 0 deletions src/test/repl/helpers/macros.ts
Original file line number Diff line number Diff line change
@@ -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);
}
3 changes: 3 additions & 0 deletions src/test/repl/helpers/misc.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { tsNodeTypes, tsSupportsMtsCtsExtensions } from '../../helpers';

export const replFile = tsSupportsMtsCtsExtensions ? '<repl>.cts' : '<repl>.ts';
13 changes: 7 additions & 6 deletions src/test/repl/repl-environment.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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],
Expand Down Expand Up @@ -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],
Expand Down Expand Up @@ -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:`
),
},
});
Expand Down Expand Up @@ -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,
},
Expand Down
Loading