From 2e3937047fd030bf46239086caca806dc67c8628 Mon Sep 17 00:00:00 2001 From: Andy Stanberry Date: Tue, 28 Sep 2021 19:11:52 -0400 Subject: [PATCH 1/8] failing swc transpiler repl test --- src/test/repl/repl.spec.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/test/repl/repl.spec.ts b/src/test/repl/repl.spec.ts index 6ec33f434..d3b03a83d 100644 --- a/src/test/repl/repl.spec.ts +++ b/src/test/repl/repl.spec.ts @@ -30,6 +30,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'); From 93f8be571cbebd789b764d850c5de534af5200ec Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Sun, 7 Nov 2021 15:44:34 -0500 Subject: [PATCH 2/8] Fix --- src/index.ts | 3 ++ src/repl.ts | 24 +++++++------ src/test/repl/repl.spec.ts | 73 +++++++++++++++++++++++++------------- 3 files changed, 66 insertions(+), 34 deletions(-) diff --git a/src/index.ts b/src/index.ts index 1fc07cd88..b3a145c6d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -471,6 +471,8 @@ export interface Service { installSourceMapSupport(): void; /** @internal */ enableExperimentalEsmLoaderInterop(): void; + /** @internal */ + transpileOnly: boolean; } /** @@ -1323,6 +1325,7 @@ export function create(rawOptions: CreateOptions = {}): Service { addDiagnosticFilter, installSourceMapSupport, enableExperimentalEsmLoaderInterop, + transpileOnly, }; } diff --git a/src/repl.ts b/src/repl.ts index ad399b850..131a03d6c 100644 --- a/src/repl.ts +++ b/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(); diff --git a/src/test/repl/repl.spec.ts b/src/test/repl/repl.spec.ts index 2ef41d15d..c0e9d3577 100644 --- a/src/test/repl/repl.spec.ts +++ b/src/test/repl/repl.spec.ts @@ -412,29 +412,54 @@ test.suite( } ); -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(''); + }); }); From 561b6ddd4a5ca873edf3062068623583a32be128 Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Mon, 27 Dec 2021 23:05:21 +0000 Subject: [PATCH 3/8] Strip sourcemap comment from REPL JS before diffing and executing --- src/repl.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/repl.ts b/src/repl.ts index ad399b850..34332b8f5 100644 --- a/src/repl.ts +++ b/src/repl.ts @@ -480,6 +480,8 @@ export function createEvalAwarePartialHost( return { readFile, fileExists }; } +const sourcemapCommentRe = /\/\/# ?sourceMappingURL=\S+[\s\r\n]*$/; + type AppendCompileAndEvalInputResult = | { containsTopLevelAwait: true; valuePromise: Promise } | { containsTopLevelAwait: false; value: any }; @@ -525,8 +527,14 @@ function appendCompileAndEvalInput(options: { output = adjustUseStrict(output); + 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(); From 339df1d669b4f538f59db82bc68d9a86a4f84363 Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Mon, 27 Dec 2021 23:12:09 +0000 Subject: [PATCH 4/8] add comment about how repl does not do sourcemaps --- src/repl.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/repl.ts b/src/repl.ts index 34332b8f5..a0f5e78b5 100644 --- a/src/repl.ts +++ b/src/repl.ts @@ -527,6 +527,12 @@ 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, From 82152ebfce2314111054f727688fdfd899ddf4b7 Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Mon, 27 Dec 2021 23:12:44 +0000 Subject: [PATCH 5/8] lint-fix --- src/repl.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/repl.ts b/src/repl.ts index a0f5e78b5..e402f3df7 100644 --- a/src/repl.ts +++ b/src/repl.ts @@ -540,7 +540,10 @@ function appendCompileAndEvalInput(options: { ); // Use `diff` to check for new JavaScript to execute. - const changes = diffLines(oldOutputWithoutSourcemapComment, outputWithoutSourcemapComment); + const changes = diffLines( + oldOutputWithoutSourcemapComment, + outputWithoutSourcemapComment + ); if (isCompletion) { undo(); From 4f832e1858c4417a9af0ef9a66e3b1d0765e55ed Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Fri, 21 Jan 2022 02:05:06 -0500 Subject: [PATCH 6/8] fix --- src/test/repl/repl.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/repl/repl.spec.ts b/src/test/repl/repl.spec.ts index c0e9d3577..7bd9b4033 100644 --- a/src/test/repl/repl.spec.ts +++ b/src/test/repl/repl.spec.ts @@ -459,7 +459,7 @@ test.suite('REPL declares types for node built-ins within REPL', (test) => { ); // Assert that we do not get errors about `declare import` syntax from swc - expect(stdout).toBe("> undefined\n> undefined\n> 'done'\n> "); + expect(stdout).toBe("> undefined\n> undefined\n> 'done'\n"); expect(stderr).toBe(''); }); }); From 95ccf721133ade104f9b5eb09f9639b3f83e6fa5 Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Fri, 21 Jan 2022 02:06:20 -0500 Subject: [PATCH 7/8] fix --- src/test/repl/repl.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/repl/repl.spec.ts b/src/test/repl/repl.spec.ts index e838e5ae7..1d326f408 100644 --- a/src/test/repl/repl.spec.ts +++ b/src/test/repl/repl.spec.ts @@ -39,7 +39,7 @@ test('should echo a value when using the swc transpiler', async () => { execPromise.child.stdin!.end('400\n401\n'); const { err, stdout } = await execPromise; expect(err).toBe(null); - expect(stdout).toBe('> 400\n>401\n > '); + expect(stdout).toBe('> 400\n> 401\n> '); }); test('REPL has command to get type information', async () => { From 9fded482814699f296f92fe95399fca861723dac Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Fri, 21 Jan 2022 09:24:33 -0500 Subject: [PATCH 8/8] sneaking in a fix for a windows test flake --- src/test/repl/repl.spec.ts | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/test/repl/repl.spec.ts b/src/test/repl/repl.spec.ts index 4f36981d1..acce2639e 100644 --- a/src/test/repl/repl.spec.ts +++ b/src/test/repl/repl.spec.ts @@ -56,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