Skip to content
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

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

Merged
merged 14 commits into from
Jan 21, 2022
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
3 changes: 3 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,8 @@ export interface Service {
installSourceMapSupport(): void;
/** @internal */
enableExperimentalEsmLoaderInterop(): void;
/** @internal */
transpileOnly: boolean;
}

/**
Expand Down Expand Up @@ -1330,6 +1332,7 @@ export function create(rawOptions: CreateOptions = {}): Service {
addDiagnosticFilter,
installSourceMapSupport,
enableExperimentalEsmLoaderInterop,
transpileOnly,
};
}

Expand Down
43 changes: 32 additions & 11 deletions src/repl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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 };
Expand Down Expand Up @@ -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();
Expand Down
103 changes: 71 additions & 32 deletions src/test/repl/repl.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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
Expand Down Expand Up @@ -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('');
});
});