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

Declare types for node builtin modules in REPL so you do not need to import them #1500

Merged
merged 3 commits into from Oct 10, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 9 additions & 9 deletions src/index.ts
Expand Up @@ -672,6 +672,15 @@ export function create(rawOptions: CreateOptions = {}): Service {
});
}

/**
* True if require() hooks should interop with experimental ESM loader.
* Enabled explicitly via a flag since it is a breaking change.
*/
let experimentalEsmLoader = false;
function enableExperimentalEsmLoaderInterop() {
experimentalEsmLoader = true;
}

// Install source map support and read from memory cache.
installSourceMapSupport();
function installSourceMapSupport() {
Expand Down Expand Up @@ -1254,15 +1263,6 @@ export function create(rawOptions: CreateOptions = {}): Service {
});
}

/**
* True if require() hooks should interop with experimental ESM loader.
* Enabled explicitly via a flag since it is a breaking change.
*/
let experimentalEsmLoader = false;
function enableExperimentalEsmLoaderInterop() {
experimentalEsmLoader = true;
}

return {
[TS_NODE_SERVICE_BRAND]: true,
ts,
Expand Down
17 changes: 17 additions & 0 deletions src/repl.ts
Expand Up @@ -14,6 +14,7 @@ import { Console } from 'console';
import * as assert from 'assert';
import type * as tty from 'tty';
import type * as Module from 'module';
import { builtinModules } from 'module';

// Lazy-loaded.
let _processTopLevelAwait: (src: string) => string | null;
Expand Down Expand Up @@ -356,6 +357,22 @@ export function createRepl(options: CreateReplOptions = {}) {
if (forceToBeModule) {
state.input += 'export {};void 0;\n';
}

// Declare node builtins.
// Skip the same builtins as `addBuiltinLibsToObject`:
// 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`;
}

reset();
Expand Down
2 changes: 2 additions & 0 deletions src/test/helpers.ts
Expand Up @@ -40,6 +40,8 @@ export const CMD_ESM_LOADER_WITHOUT_PROJECT = `node ${EXPERIMENTAL_MODULES_FLAG}
// `createRequire` does not exist on older node versions
export const testsDirRequire = createRequire(join(TEST_DIR, 'index.js'));

export const ts = testsDirRequire('typescript');

export const xfs = new NodeFS(fs);

/** Pass to `test.context()` to get access to the ts-node API under test */
Expand Down
2 changes: 1 addition & 1 deletion src/test/index.spec.ts
Expand Up @@ -3,7 +3,7 @@ import * as expect from 'expect';
import { join, resolve, sep as pathSep } from 'path';
import { tmpdir } from 'os';
import semver = require('semver');
import ts = require('typescript');
import { ts } from './helpers';
import { lstatSync, mkdtempSync } from 'fs';
import { npath } from '@yarnpkg/fslib';
import type _createRequire from 'create-require';
Expand Down
2 changes: 1 addition & 1 deletion src/test/repl/node-repl-tla.ts
Expand Up @@ -2,7 +2,7 @@ import { expect } from 'chai';
import type { Key } from 'readline';
import { Stream } from 'stream';
import semver = require('semver');
import ts = require('typescript');
import { ts } from '../helpers';
import type { ContextWithTsNodeUnderTest } from './helpers';

interface SharedObjects extends ContextWithTsNodeUnderTest {
Expand Down
8 changes: 4 additions & 4 deletions src/test/repl/repl-environment.spec.ts
Expand Up @@ -94,7 +94,7 @@ test.suite(
}
);

const declareGlobals = `declare var replReport: any, stdinReport: any, evalReport: any, restReport: any, global: any, __filename: any, __dirname: any, module: any, exports: any, fs: any;`;
const declareGlobals = `declare var replReport: any, stdinReport: any, evalReport: any, restReport: any, global: any, __filename: any, __dirname: any, module: any, exports: any;`;
function setReportGlobal(type: 'repl' | 'stdin' | 'eval') {
return `
${declareGlobals}
Expand All @@ -107,7 +107,7 @@ test.suite(
modulePaths: typeof module !== 'undefined' && [...module.paths],
exportsTest: typeof exports !== 'undefined' ? module.exports === exports : null,
stackTest: new Error().stack!.split('\\n')[1],
moduleAccessorsTest: typeof fs === 'undefined' ? null : fs === require('fs'),
moduleAccessorsTest: eval('typeof fs') === 'undefined' ? null : eval('fs') === require('fs'),
argv: [...process.argv]
};
`.replace(/\n/g, '');
Expand Down Expand Up @@ -203,7 +203,7 @@ test.suite(
exportsTest: true,
// Note: vanilla node uses different name. See #1360
stackTest: expect.stringContaining(
` at ${join(TEST_DIR, '<repl>.ts')}:2:`
` at ${join(TEST_DIR, '<repl>.ts')}:4:`
),
moduleAccessorsTest: true,
argv: [tsNodeExe],
Expand Down Expand Up @@ -356,7 +356,7 @@ test.suite(
exportsTest: true,
// Note: vanilla node uses different name. See #1360
stackTest: expect.stringContaining(
` at ${join(TEST_DIR, '<repl>.ts')}:2:`
` at ${join(TEST_DIR, '<repl>.ts')}:4:`
),
moduleAccessorsTest: true,
argv: [tsNodeExe],
Expand Down
31 changes: 29 additions & 2 deletions src/test/repl/repl.spec.ts
@@ -1,4 +1,4 @@
import ts = require('typescript');
import { ts } from '../helpers';
import semver = require('semver');
import * as expect from 'expect';
import {
Expand Down Expand Up @@ -212,7 +212,7 @@ test.suite('top level await', (_test) => {

expect(stdout).toBe('> > ');
expect(stderr.replace(/\r\n/g, '\n')).toBe(
'<repl>.ts(2,7): error TS2322: ' +
'<repl>.ts(4,7): error TS2322: ' +
(semver.gte(ts.version, '4.0.0')
? `Type 'number' is not assignable to type 'string'.\n`
: `Type '1' is not assignable to type 'string'.\n`) +
Expand Down Expand Up @@ -411,3 +411,30 @@ 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,
},
}
);

// 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`);
});