From 3e1e546bb852dbb818fccc3e607bf17748dc6dbe Mon Sep 17 00:00:00 2001 From: Hiroki Osame Date: Sat, 29 Jun 2024 15:04:40 +0900 Subject: [PATCH] feat(cjs): improve compatibility with other loaders --- package.json | 1 + pnpm-lock.yaml | 36 ++++++++++++++++++++ src/cjs/api/module-extensions.ts | 46 ++++++++++++++------------ src/cjs/api/module-resolve-filename.ts | 7 +++- src/cjs/api/register.ts | 27 ++++++++++----- src/cjs/api/types.ts | 4 +++ tests/specs/api.ts | 34 ++++++++++++++++++- 7 files changed, 123 insertions(+), 32 deletions(-) diff --git a/package.json b/package.json index 543f5b1a..29909f90 100644 --- a/package.json +++ b/package.json @@ -108,6 +108,7 @@ "node-pty": "^1.0.0", "outdent": "^0.8.0", "pkgroll": "^2.1.1", + "proxyquire": "^2.1.3", "simple-git-hooks": "^2.11.1", "split2": "^4.2.0", "strip-ansi": "^7.1.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 53b933e8..bacbf2c5 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -91,6 +91,9 @@ importers: pkgroll: specifier: ^2.1.1 version: 2.1.1(typescript@5.4.5) + proxyquire: + specifier: ^2.1.3 + version: 2.1.3 simple-git-hooks: specifier: ^2.11.1 version: 2.11.1 @@ -1856,6 +1859,10 @@ packages: resolution: {integrity: sha512-7Gps/XWymbLk2QLYK4NzpMOrYjMhdIxXuIvy2QBsLE6ljuodKvdkWs/cpyJJ3CVIVpH0Oi1Hvg1ovbMzLdFBBg==} engines: {node: ^10.12.0 || >=12.0.0} + fill-keys@1.0.2: + resolution: {integrity: sha512-tcgI872xXjwFF4xgQmLxi76GnwJG3g/3isB1l4/G5Z4zrbddGpBjqZCO9oEAcB5wX0Hj/5iQB3toxfO7in1hHA==} + engines: {node: '>=0.10.0'} + fill-range@7.0.1: resolution: {integrity: sha512-qOo9F+dMUmC2Lcb4BbVvnKJxTPjCm+RRpe4gDuGrzkL7mEVl/djYSu2OdQ2Pa302N4oqkSg9ir6jaLWJ2USVpQ==} engines: {node: '>=8'} @@ -2213,6 +2220,9 @@ packages: is-obj-prop@1.0.0: resolution: {integrity: sha512-5Idb61slRlJlsAzi0Wsfwbp+zZY+9LXKUAZpvT/1ySw+NxKLRWfa0Bzj+wXI3fX5O9hiddm5c3DAaRSNP/yl2w==} + is-object@1.0.2: + resolution: {integrity: sha512-2rRIahhZr2UWb45fIOuvZGpFtz0TyOZLf32KxBbSoUCeZR495zCKlWUKKUByk3geS2eAs7ZAABt0Y/Rx0GiQGA==} + is-path-inside@3.0.3: resolution: {integrity: sha512-Fd4gABb+ycGAmKou8eMftCupSir5lRxqf4aD/vd0cD2qc4HL07OjCeuHMr8Ro4CoMaeCKDB0/ECBOVWjTwUvPQ==} engines: {node: '>=8'} @@ -2471,6 +2481,9 @@ packages: resolution: {integrity: sha512-f16coDZlTG1jskq3mxarwB+fGRrd0uXWt+o1WIhRfOwbXQZqUDsTVxQBFK9JjRQHblg8eAG2JSbprDXKjc7ijQ==} engines: {node: '>= 4.0.0'} + merge-descriptors@1.0.3: + resolution: {integrity: sha512-gaNvAS7TZ897/rVaZ0nMtAyxNyi/pdbjbAwUpFQpN70GqnVfOiXpeUUMKRBmzXaSQ8DdTX4/0ms62r2K+hE6mQ==} + merge-stream@2.0.0: resolution: {integrity: sha512-abv/qOcuPfk3URPfDzmZU1LKmuw8kT+0nIHvKrKgFrwifol/doWcdA4ZqsWQ8ENrFKkd67Mfpo/LovbIUsbt3w==} @@ -2540,6 +2553,9 @@ packages: mkdirp-classic@0.5.3: resolution: {integrity: sha512-gKLcREMhtuZRwRAfqP3RFW+TK4JqApVBtOIftVgjuABpAtpxhPGaDcfvbhNvD0B8iD1oUr/txX35NjcaY6Ns/A==} + module-not-found-error@1.0.1: + resolution: {integrity: sha512-pEk4ECWQXV6z2zjhRZUongnLJNUeGQJ3w6OQ5ctGwD+i5o93qjRQUk2Rt6VdNeu3sEP0AB4LcfvdebpxBRVr4g==} + move-file@3.1.0: resolution: {integrity: sha512-4aE3U7CCBWgrQlQDMq8da4woBWDGHioJFiOZ8Ie6Yq2uwYQ9V2kGhTz4x3u6Wc+OU17nw0yc3rJ/lQ4jIiPe3A==} engines: {node: ^12.20.0 || ^14.13.1 || >=16.0.0} @@ -2876,6 +2892,9 @@ packages: resolution: {integrity: sha512-2yma2tog9VaRZY2mn3Wq51uiSW4NcPYT1cQdBagwyrznrilKSZwIZ0UG3ZPL/mx+axEns0hE35T5ufOYZXEnBQ==} engines: {node: '>=4'} + proxyquire@2.1.3: + resolution: {integrity: sha512-BQWfCqYM+QINd+yawJz23tbBM40VIGXOdDw3X344KcclI/gtBbdWF6SlQ4nK/bYhF9d27KYug9WzljHC6B9Ysg==} + pump@3.0.0: resolution: {integrity: sha512-LwZy+p3SFs1Pytd/jYct4wpv49HiYCqd9Rlc5ZVdk0V+8Yzv6jR5Blk3TRmPL1ft69TxP0IMZGJ+WPFU2BFhww==} @@ -5414,6 +5433,11 @@ snapshots: dependencies: flat-cache: 3.2.0 + fill-keys@1.0.2: + dependencies: + is-object: 1.0.2 + merge-descriptors: 1.0.3 + fill-range@7.0.1: dependencies: to-regex-range: 5.0.1 @@ -5782,6 +5806,8 @@ snapshots: lowercase-keys: 1.0.1 obj-props: 1.4.0 + is-object@1.0.2: {} + is-path-inside@3.0.3: {} is-plain-obj@4.1.0: {} @@ -6098,6 +6124,8 @@ snapshots: sonic-forest: 1.0.0(tslib@2.6.2) tslib: 2.6.2 + merge-descriptors@1.0.3: {} + merge-stream@2.0.0: {} merge2@1.4.1: {} @@ -6155,6 +6183,8 @@ snapshots: mkdirp-classic@0.5.3: {} + module-not-found-error@1.0.1: {} + move-file@3.1.0: dependencies: path-exists: 5.0.0 @@ -6475,6 +6505,12 @@ snapshots: proto-props@2.0.0: {} + proxyquire@2.1.3: + dependencies: + fill-keys: 1.0.2 + module-not-found-error: 1.0.1 + resolve: 1.22.8 + pump@3.0.0: dependencies: end-of-stream: 1.4.4 diff --git a/src/cjs/api/module-extensions.ts b/src/cjs/api/module-extensions.ts index f41b74a1..654c992d 100644 --- a/src/cjs/api/module-extensions.ts +++ b/src/cjs/api/module-extensions.ts @@ -8,6 +8,7 @@ import { shouldApplySourceMap, inlineSourceMap } from '../../source-map.js'; import { parent } from '../../utils/ipc/client.js'; import { fileMatcher } from '../../utils/tsconfig.js'; import { implicitlyResolvableExtensions } from './resolve-implicit-extensions.js'; +import type { LoaderState } from './types.js'; const typescriptExtensions = [ '.cts', @@ -23,22 +24,6 @@ const transformExtensions = [ '.mjs', ] as const; -const cloneExtensions = ( - extensions: ObjectType, -) => { - const cloneTo: ObjectType = Object.create(Object.getPrototypeOf(extensions)); - - // Preserves setters if they exist (e.g. nyc via append-transform) - const descriptors = Object.getOwnPropertyDescriptors(extensions); - for (const property in descriptors) { - if (Object.hasOwn(descriptors, property)) { - Object.defineProperty(cloneTo, property, descriptors[property]); - } - } - - return cloneTo; -}; - const safeSet = >( object: T, property: keyof T, @@ -82,18 +67,20 @@ const safeSet = >( }; export const createExtensions = ( - extendExtensions: NodeJS.RequireExtensions, + state: LoaderState, + extensions: NodeJS.RequireExtensions, namespace?: string, ) => { - // Clone Module._extensions with null prototype - const extensions = cloneExtensions(extendExtensions); - const defaultLoader = extensions['.js']; const transformer = ( module: Module, filePath: string, ) => { + if (state.enabled === false) { + return defaultLoader(module, filePath); + } + // Make sure __filename doesnt contain query const [cleanFilePath, query] = filePath.split('?'); @@ -198,5 +185,22 @@ export const createExtensions = ( configurable: true, }); - return extensions; + // Unregister + return () => { + /** + * The extensions are only reverted if they're still tsx's transformers + * + * Otherwise, it means they have been wrapped by another loader and should + * be left untouched not to remove the other loader + */ + if (extensions['.js'] === transformer) { + extensions['.js'] = defaultLoader; + } + + for (const extension of [...implicitlyResolvableExtensions, '.mjs']) { + if (extensions[extension] === transformer) { + delete extensions[extension]; + } + } + }; }; diff --git a/src/cjs/api/module-resolve-filename.ts b/src/cjs/api/module-resolve-filename.ts index 3b336ace..c3b91248 100644 --- a/src/cjs/api/module-resolve-filename.ts +++ b/src/cjs/api/module-resolve-filename.ts @@ -6,7 +6,7 @@ import type { NodeError } from '../../types.js'; import { isRelativePath, fileUrlPrefix, tsExtensionsPattern } from '../../utils/path-utils.js'; import { tsconfigPathsMatcher, allowJs } from '../../utils/tsconfig.js'; import { urlSearchParamsStringify } from '../../utils/url-search-params-stringify.js'; -import type { ResolveFilename, SimpleResolve } from './types.js'; +import type { ResolveFilename, SimpleResolve, LoaderState } from './types.js'; import { createImplicitResolver } from './resolve-implicit-extensions.js'; const nodeModulesPath = `${path.sep}node_modules${path.sep}`; @@ -154,6 +154,7 @@ const resolveRequest = ( }; export const createResolveFilename = ( + state: LoaderState, nextResolve: ResolveFilename, namespace?: string, ): ResolveFilename => ( @@ -162,6 +163,10 @@ export const createResolveFilename = ( isMain, options, ) => { + if (state.enabled === false) { + return nextResolve(request, parent, isMain, options); + } + const resolve: SimpleResolve = request_ => nextResolve( request_, parent, diff --git a/src/cjs/api/register.ts b/src/cjs/api/register.ts index e2b9db97..8c08646b 100644 --- a/src/cjs/api/register.ts +++ b/src/cjs/api/register.ts @@ -4,6 +4,7 @@ import { fileURLToPath } from 'node:url'; import { loadTsconfig } from '../../utils/tsconfig.js'; import type { RequiredProperty } from '../../types.js'; import { urlSearchParamsStringify } from '../../utils/url-search-params-stringify.js'; +import type { LoaderState } from './types.js'; import { createExtensions } from './module-extensions.js'; import { createResolveFilename } from './module-resolve-filename.js'; @@ -62,27 +63,35 @@ export const register: Register = ( options, ) => { const { sourceMapsEnabled } = process; - const { _extensions, _resolveFilename } = Module; + const state: LoaderState = { + enabled: true, + }; loadTsconfig(process.env.TSX_TSCONFIG_PATH); // register process.setSourceMapsEnabled(true); - const resolveFilename = createResolveFilename(_resolveFilename, options?.namespace); + + const originalResolveFilename = Module._resolveFilename; + const resolveFilename = createResolveFilename(state, originalResolveFilename, options?.namespace); Module._resolveFilename = resolveFilename; - const extensions = createExtensions(Module._extensions, options?.namespace); - // @ts-expect-error overwriting read-only property - Module._extensions = extensions; + const unregisterExtensions = createExtensions(state, Module._extensions, options?.namespace); const unregister = () => { if (sourceMapsEnabled === false) { process.setSourceMapsEnabled(false); } - - // @ts-expect-error overwriting read-only property - Module._extensions = _extensions; - Module._resolveFilename = _resolveFilename; + state.enabled = false; + + /** + * Only revert the _resolveFilename & extensions if they're unwrapped + * by another loader extension + */ + if (Module._resolveFilename === resolveFilename) { + Module._resolveFilename = originalResolveFilename; + } + unregisterExtensions(); }; if (options?.namespace) { diff --git a/src/cjs/api/types.ts b/src/cjs/api/types.ts index b0eacaab..3e9ba2d5 100644 --- a/src/cjs/api/types.ts +++ b/src/cjs/api/types.ts @@ -1,5 +1,9 @@ import type Module from 'module'; +export type LoaderState = { + enabled: boolean; +}; + export type ResolveFilename = typeof Module._resolveFilename; export type SimpleResolve = (request: string) => string; diff --git a/tests/specs/api.ts b/tests/specs/api.ts index b66bb483..fd11668c 100644 --- a/tests/specs/api.ts +++ b/tests/specs/api.ts @@ -118,7 +118,7 @@ export default testSuite(({ describe }, node: NodeApis) => { return code; }, '.ts'); `, - node_modules: ({ symlink }) => symlink(path.resolve('node_modules'), 'junction'), + 'node_modules/append-transform': ({ symlink }) => symlink(path.resolve('node_modules/append-transform'), 'junction'), }); const { stdout } = await execaNode('./index.js', { @@ -311,6 +311,38 @@ export default testSuite(({ describe }, node: NodeApis) => { expect(stdout).toBe('foo bar json file.ts\nfoo bar json file.ts\nfoo bar json file.ts\nUnregistered'); }); + + test('works with proxyquire (eslint tests)', async () => { + await using fixture = await createFixture({ + 'index.js': ` + const proxyquire = require('proxyquire'); + const tsx = require(${JSON.stringify(tsxCjsApiPath)}); + + tsx.register(); + + proxyquire('./test.js', { + path: { + sep: 'hello world', + }, + }); + `, + + 'test.js': ` + const path = require('path'); + console.log(path.sep); + `, + + 'node_modules/proxyquire': ({ symlink }) => symlink(path.resolve('node_modules/proxyquire'), 'junction'), + }); + + const { stdout } = await execaNode('./index.js', { + cwd: fixture.path, + nodePath: node.path, + nodeOptions: [], + }); + + expect(stdout).toBe('hello world'); + }); }); });