From b52ca45a1d8b1178640585417ed5e703f6722b2f Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Sun, 10 Oct 2021 13:53:46 -0400 Subject: [PATCH] Allow composing `register(create())`; refactor tests (#1474) * WIP add ability to compose register(create()) * wip * Fix environmental reset in tests * fix * fix * Fix * fix * fix * fix * fix --- .vscode/launch.json | 16 ++++ .vscode/tasks.json | 13 +++ package.json | 1 + src/index.ts | 90 +++++++++++------- src/test/helpers.ts | 66 ++++++++++++- src/test/register.spec.ts | 189 +++++++++++++++++++++----------------- src/test/testlib.ts | 4 +- 7 files changed, 257 insertions(+), 122 deletions(-) create mode 100644 .vscode/launch.json create mode 100644 .vscode/tasks.json diff --git a/.vscode/launch.json b/.vscode/launch.json new file mode 100644 index 000000000..8b1ffa3f4 --- /dev/null +++ b/.vscode/launch.json @@ -0,0 +1,16 @@ +{ + "configurations": [ + { + "name": "Debug AVA test file", + "type": "node", + "request": "launch", + "preLaunchTask": "npm: pre-debug", + "runtimeExecutable": "${workspaceFolder}/node_modules/.bin/ava", + "program": "${file}", + "outputCapture": "std", + "skipFiles": [ + "/**/*.js" + ], + } + ], +} diff --git a/.vscode/tasks.json b/.vscode/tasks.json new file mode 100644 index 000000000..b39ad703a --- /dev/null +++ b/.vscode/tasks.json @@ -0,0 +1,13 @@ +{ + "tasks": [ + { + "type": "npm", + "script": "pre-debug", + "problemMatcher": [ + "$tsc" + ], + "label": "npm: pre-debug", + "detail": "npm run build-tsc && npm run build-pack" + } + ] +} diff --git a/package.json b/package.json index e12c6e74b..f00637cbc 100644 --- a/package.json +++ b/package.json @@ -67,6 +67,7 @@ "test-cov": "nyc ava", "test": "npm run build && npm run lint && npm run test-cov --", "test-local": "npm run lint-fix && npm run build-tsc && npm run build-pack && npm run test-spec --", + "pre-debug": "npm run build-tsc && npm run build-pack", "coverage-report": "nyc report --reporter=lcov", "prepare": "npm run clean && npm run build-nopack", "api-extractor": "api-extractor run --local --verbose" diff --git a/src/index.ts b/src/index.ts index f93005eea..dee5fa1b4 100644 --- a/src/index.ts +++ b/src/index.ts @@ -427,10 +427,14 @@ export class TSError extends BaseError { } } +const TS_NODE_SERVICE_BRAND = Symbol('TS_NODE_SERVICE_BRAND'); + /** * Primary ts-node service, which wraps the TypeScript API and can compile TypeScript to JavaScript */ export interface Service { + /** @internal */ + [TS_NODE_SERVICE_BRAND]: true; ts: TSCommon; config: _ts.ParsedCommandLine; options: RegisterOptions; @@ -446,6 +450,8 @@ export interface Service { readonly shouldReplAwait: boolean; /** @internal */ addDiagnosticFilter(filter: DiagnosticFilter): void; + /** @internal */ + installSourceMapSupport(): void; } /** @@ -477,12 +483,25 @@ export function getExtensions(config: _ts.ParsedCommandLine) { return { tsExtensions, jsExtensions }; } +/** + * Create a new TypeScript compiler instance and register it onto node.js + */ +export function register(opts?: RegisterOptions): Service; /** * Register TypeScript compiler instance onto node.js */ -export function register(opts: RegisterOptions = {}): Service { +export function register(service: Service): Service; +export function register( + serviceOrOpts: Service | RegisterOptions | undefined +): Service { + // Is this a Service or a RegisterOptions? + let service = serviceOrOpts as Service; + if (!(serviceOrOpts as Service)?.[TS_NODE_SERVICE_BRAND]) { + // Not a service; is options + service = create((serviceOrOpts ?? {}) as RegisterOptions); + } + const originalJsHandler = require.extensions['.js']; - const service = create(opts); const { tsExtensions, jsExtensions } = getExtensions(service.config); const extensions = [...tsExtensions, ...jsExtensions]; @@ -660,38 +679,41 @@ export function create(rawOptions: CreateOptions = {}): Service { } // Install source map support and read from memory cache. - sourceMapSupport.install({ - environment: 'node', - retrieveFile(pathOrUrl: string) { - let path = pathOrUrl; - // If it's a file URL, convert to local path - // Note: fileURLToPath does not exist on early node v10 - // I could not find a way to handle non-URLs except to swallow an error - if (options.experimentalEsmLoader && path.startsWith('file://')) { - try { - path = fileURLToPath(path); - } catch (e) { - /* swallow error */ + installSourceMapSupport(); + function installSourceMapSupport() { + sourceMapSupport.install({ + environment: 'node', + retrieveFile(pathOrUrl: string) { + let path = pathOrUrl; + // If it's a file URL, convert to local path + // Note: fileURLToPath does not exist on early node v10 + // I could not find a way to handle non-URLs except to swallow an error + if (options.experimentalEsmLoader && path.startsWith('file://')) { + try { + path = fileURLToPath(path); + } catch (e) { + /* swallow error */ + } } - } - path = normalizeSlashes(path); - return outputCache.get(path)?.content || ''; - }, - redirectConflictingLibrary: true, - onConflictingLibraryRedirect( - request, - parent, - isMain, - options, - redirectedRequest - ) { - debug( - `Redirected an attempt to require source-map-support to instead receive @cspotcode/source-map-support. "${ - (parent as NodeJS.Module).filename - }" attempted to require or resolve "${request}" and was redirected to "${redirectedRequest}".` - ); - }, - }); + path = normalizeSlashes(path); + return outputCache.get(path)?.content || ''; + }, + redirectConflictingLibrary: true, + onConflictingLibraryRedirect( + request, + parent, + isMain, + options, + redirectedRequest + ) { + debug( + `Redirected an attempt to require source-map-support to instead receive @cspotcode/source-map-support. "${ + (parent as NodeJS.Module).filename + }" attempted to require or resolve "${request}" and was redirected to "${redirectedRequest}".` + ); + }, + }); + } const shouldHavePrettyErrors = options.pretty === undefined ? process.stdout.isTTY : options.pretty; @@ -1239,6 +1261,7 @@ export function create(rawOptions: CreateOptions = {}): Service { } return { + [TS_NODE_SERVICE_BRAND]: true, ts, config, compile, @@ -1250,6 +1273,7 @@ export function create(rawOptions: CreateOptions = {}): Service { moduleTypeClassifier, shouldReplAwait, addDiagnosticFilter, + installSourceMapSupport, }; } diff --git a/src/test/helpers.ts b/src/test/helpers.ts index 6683b0558..00b59d12f 100644 --- a/src/test/helpers.ts +++ b/src/test/helpers.ts @@ -12,9 +12,9 @@ import type { Readable } from 'stream'; */ import type * as tsNodeTypes from '../index'; import type _createRequire from 'create-require'; -import { once } from 'lodash'; +import { has, once } from 'lodash'; import semver = require('semver'); -import { isConstructSignatureDeclaration } from 'typescript'; +import * as expect from 'expect'; const createRequire: typeof _createRequire = require('create-require'); export { tsNodeTypes }; @@ -45,7 +45,7 @@ export const xfs = new NodeFS(fs); /** Pass to `test.context()` to get access to the ts-node API under test */ export const contextTsNodeUnderTest = once(async () => { await installTsNode(); - const tsNodeUnderTest = testsDirRequire('ts-node'); + const tsNodeUnderTest: typeof tsNodeTypes = testsDirRequire('ts-node'); return { tsNodeUnderTest, }; @@ -155,3 +155,63 @@ export function getStream(stream: Readable, waitForPattern?: string | RegExp) { combinedString = combinedBuffer.toString('utf8'); } } + +const defaultRequireExtensions = captureObjectState(require.extensions); +const defaultProcess = captureObjectState(process); +const defaultModule = captureObjectState(require('module')); +const defaultError = captureObjectState(Error); +const defaultGlobal = captureObjectState(global); + +/** + * Undo all of ts-node & co's installed hooks, resetting the node environment to default + * so we can run multiple test cases which `.register()` ts-node. + * + * Must also play nice with `nyc`'s environmental mutations. + */ +export function resetNodeEnvironment() { + // We must uninstall so that it resets its internal state; otherwise it won't know it needs to reinstall in the next test. + require('@cspotcode/source-map-support').uninstall(); + + // Modified by ts-node hooks + resetObject(require.extensions, defaultRequireExtensions); + + // ts-node attaches a property when it registers an instance + // source-map-support monkey-patches the emit function + resetObject(process, defaultProcess); + + // source-map-support swaps out the prepareStackTrace function + resetObject(Error, defaultError); + + // _resolveFilename is modified by tsconfig-paths, future versions of source-map-support, and maybe future versions of ts-node + resetObject(require('module'), defaultModule); + + // May be modified by REPL tests, since the REPL sets globals. + resetObject(global, defaultGlobal); +} + +function captureObjectState(object: any) { + return { + descriptors: Object.getOwnPropertyDescriptors(object), + values: { ...object }, + }; +} +// Redefine all property descriptors and delete any new properties +function resetObject( + object: any, + state: ReturnType +) { + const currentDescriptors = Object.getOwnPropertyDescriptors(object); + for (const key of Object.keys(currentDescriptors)) { + if (!has(state.descriptors, key)) { + delete object[key]; + } + } + // Trigger nyc's setter functions + for (const [key, value] of Object.entries(state.values)) { + try { + object[key] = value; + } catch {} + } + // Reset descriptors + Object.defineProperties(object, state.descriptors); +} diff --git a/src/test/register.spec.ts b/src/test/register.spec.ts index 5708b9c37..db04b2846 100644 --- a/src/test/register.spec.ts +++ b/src/test/register.spec.ts @@ -1,45 +1,72 @@ import { once } from 'lodash'; import { - installTsNode, + contextTsNodeUnderTest, PROJECT, - testsDirRequire, + resetNodeEnvironment, TEST_DIR, tsNodeTypes, } from './helpers'; -import { test } from './testlib'; +import { context } from './testlib'; import { expect } from 'chai'; -import { join } from 'path'; +import * as exp from 'expect'; +import { join, resolve } from 'path'; import proxyquire = require('proxyquire'); -import type * as Module from 'module'; const SOURCE_MAP_REGEXP = /\/\/# sourceMappingURL=data:application\/json;charset=utf\-8;base64,[\w\+]+=*$/; -// Set after ts-node is installed locally -let { register }: typeof tsNodeTypes = {} as any; -test.beforeAll(async () => { - await installTsNode(); - ({ register } = testsDirRequire('ts-node')); +const createOptions: tsNodeTypes.CreateOptions = { + project: PROJECT, + compilerOptions: { + jsx: 'preserve', + }, +}; + +const test = context(contextTsNodeUnderTest).context( + once(async (t) => { + return { + moduleTestPath: resolve(__dirname, '../../tests/module.ts'), + service: t.context.tsNodeUnderTest.create(createOptions), + }; + }) +); +test.beforeEach(async (t) => { + // Un-install all hook and remove our test module from cache + resetNodeEnvironment(); + delete require.cache[t.context.moduleTestPath]; + // Paranoid check that we are truly uninstalled + exp(() => require(t.context.moduleTestPath)).toThrow( + "Unexpected token 'export'" + ); }); - -test.suite('register', (_test) => { - const test = _test.context( - once(async () => { - return { - registered: register({ - project: PROJECT, - compilerOptions: { - jsx: 'preserve', - }, - }), - moduleTestPath: require.resolve('../../tests/module'), - }; - }) +test.runSerially(); + +test('create() does not register()', async (t) => { + // nyc sets its own `require.extensions` hooks; to truly detect if we're + // installed we must attempt to load a TS file + t.context.tsNodeUnderTest.create(createOptions); + // This error indicates node attempted to run the code as .js + exp(() => require(t.context.moduleTestPath)).toThrow( + "Unexpected token 'export'" ); - test.beforeEach(async ({ context: { registered } }) => { +}); + +test('register(options) is shorthand for register(create(options))', (t) => { + t.context.tsNodeUnderTest.register(createOptions); + require(t.context.moduleTestPath); +}); + +test('register(service) registers a previously-created service', (t) => { + t.context.tsNodeUnderTest.register(t.context.service); + require(t.context.moduleTestPath); +}); + +test.suite('register(create(options))', (test) => { + test.beforeEach(async (t) => { // Re-enable project for every test. - registered.enabled(true); + t.context.service.enabled(true); + t.context.tsNodeUnderTest.register(t.context.service); + t.context.service.installSourceMapSupport(); }); - test.runSerially(); test('should be able to require typescript', ({ context: { moduleTestPath }, @@ -50,75 +77,29 @@ test.suite('register', (_test) => { }); test('should support dynamically disabling', ({ - context: { registered, moduleTestPath }, + context: { service, moduleTestPath }, }) => { delete require.cache[moduleTestPath]; - expect(registered.enabled(false)).to.equal(false); + expect(service.enabled(false)).to.equal(false); expect(() => require(moduleTestPath)).to.throw(/Unexpected token/); delete require.cache[moduleTestPath]; - expect(registered.enabled()).to.equal(false); + expect(service.enabled()).to.equal(false); expect(() => require(moduleTestPath)).to.throw(/Unexpected token/); delete require.cache[moduleTestPath]; - expect(registered.enabled(true)).to.equal(true); + expect(service.enabled(true)).to.equal(true); expect(() => require(moduleTestPath)).to.not.throw(); delete require.cache[moduleTestPath]; - expect(registered.enabled()).to.equal(true); + expect(service.enabled()).to.equal(true); expect(() => require(moduleTestPath)).to.not.throw(); }); - test('should support compiler scopes', ({ - context: { registered, moduleTestPath }, - }) => { - const calls: string[] = []; - - registered.enabled(false); - - const compilers = [ - register({ - projectSearchDir: join(TEST_DIR, 'scope/a'), - scopeDir: join(TEST_DIR, 'scope/a'), - scope: true, - }), - register({ - projectSearchDir: join(TEST_DIR, 'scope/a'), - scopeDir: join(TEST_DIR, 'scope/b'), - scope: true, - }), - ]; - - compilers.forEach((c) => { - const old = c.compile; - c.compile = (code, fileName, lineOffset) => { - calls.push(fileName); - - return old(code, fileName, lineOffset); - }; - }); - - try { - expect(require('../../tests/scope/a').ext).to.equal('.ts'); - expect(require('../../tests/scope/b').ext).to.equal('.ts'); - } finally { - compilers.forEach((c) => c.enabled(false)); - } - - expect(calls).to.deep.equal([ - join(TEST_DIR, 'scope/a/index.ts'), - join(TEST_DIR, 'scope/b/index.ts'), - ]); - - delete require.cache[moduleTestPath]; - - expect(() => require(moduleTestPath)).to.throw(); - }); - test('should compile through js and ts', () => { const m = require('../../tests/complex'); @@ -143,7 +124,7 @@ test.suite('register', (_test) => { try { require('../../tests/throw error'); } catch (error: any) { - expect(error.stack).to.contain( + exp(error.stack).toMatch( [ 'Error: this is a demo', ` at Foo.bar (${join(TEST_DIR, './throw error.ts')}:100:17)`, @@ -153,12 +134,10 @@ test.suite('register', (_test) => { }); test.suite('JSX preserve', (test) => { - let old: (m: Module, filename: string) => any; let compiled: string; - test.runSerially(); test.beforeAll(async () => { - old = require.extensions['.tsx']!; + const old = require.extensions['.tsx']!; require.extensions['.tsx'] = (m: any, fileName) => { const _compile = m._compile; @@ -172,9 +151,6 @@ test.suite('register', (_test) => { }); test('should use source maps', async (t) => { - t.teardown(() => { - require.extensions['.tsx'] = old; - }); try { require('../../tests/with-jsx.tsx'); } catch (error: any) { @@ -185,3 +161,46 @@ test.suite('register', (_test) => { }); }); }); + +test('should support compiler scopes w/multiple registered compiler services at once', (t) => { + const { moduleTestPath, tsNodeUnderTest } = t.context; + const calls: string[] = []; + + const compilers = [ + tsNodeUnderTest.register({ + projectSearchDir: join(TEST_DIR, 'scope/a'), + scopeDir: join(TEST_DIR, 'scope/a'), + scope: true, + }), + tsNodeUnderTest.register({ + projectSearchDir: join(TEST_DIR, 'scope/a'), + scopeDir: join(TEST_DIR, 'scope/b'), + scope: true, + }), + ]; + + compilers.forEach((c) => { + const old = c.compile; + c.compile = (code, fileName, lineOffset) => { + calls.push(fileName); + + return old(code, fileName, lineOffset); + }; + }); + + try { + expect(require('../../tests/scope/a').ext).to.equal('.ts'); + expect(require('../../tests/scope/b').ext).to.equal('.ts'); + } finally { + compilers.forEach((c) => c.enabled(false)); + } + + expect(calls).to.deep.equal([ + join(TEST_DIR, 'scope/a/index.ts'), + join(TEST_DIR, 'scope/b/index.ts'), + ]); + + delete require.cache[moduleTestPath]; + + expect(() => require(moduleTestPath)).to.throw(); +}); diff --git a/src/test/testlib.ts b/src/test/testlib.ts index 5d090d7d7..ce97a07a3 100644 --- a/src/test/testlib.ts +++ b/src/test/testlib.ts @@ -37,6 +37,8 @@ export const test = createTestInterface({ }); // In case someone wants to `const test = _test.context()` export { test as _test }; +// Or import `context` +export const context = test.context; export interface TestInterface< Context @@ -85,7 +87,7 @@ export interface TestInterface< beforeAll(cb: (t: ExecutionContext) => Promise): void; beforeEach(cb: (t: ExecutionContext) => Promise): void; - context( + context( cb: (t: ExecutionContext) => Promise ): TestInterface; suite(title: string, cb: (test: TestInterface) => void): void;