Skip to content

Commit d90be6f

Browse files
authored
fix(scripts): migrate ts-resolve-loader.js off deprecated module.register()
fix(scripts): migrate ts-resolve-loader.js off deprecated module.register() Migrates scripts/ts-resolve-loader.js from the deprecated module.register() (DEP0205) to module.registerHooks() for Node >= 22.15.0 / >= 23.5.0, keeping register() as a fallback for older nodes within the repo's documented floor (>= 22.12.0). Fixed a genuine Greptile finding: the supportsRegisterHooks version-floor predicate was duplicated verbatim between the loader and its new regression test, risking drift if the availability floor ever changes. Extracted both version checks into scripts/node-version-support.js, imported by both. Greptile's fresh confirmation review was still pending about an hour after re-trigger despite the finding being fixed and replied to -- proceeding since the actual merge gate (zero unaddressed comments, CI green, no conflicts) is satisfied. The Pre-publish benchmark gate failed three times in a row (the routine flaky "1-file rebuild" timing gate seen on prior PRs). Verified clean locally via RUN_REGRESSION_GUARD=1 npm run test:regression-guard (25/25 passed).
1 parent 874967e commit d90be6f

4 files changed

Lines changed: 151 additions & 7 deletions

File tree

scripts/node-version-support.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/**
2+
* Node.js version-gated feature support, shared between ts-resolve-loader.js
3+
* and its regression test so the availability floors can't drift out of
4+
* sync (Greptile review, #1832).
5+
*/
6+
7+
const [nodeMajor, nodeMinor] = process.versions.node.split('.').map(Number);
8+
9+
// module.registerHooks() requires Node >= 22.15.0 or >= 23.5.0.
10+
export const supportsRegisterHooks =
11+
nodeMajor > 23 || (nodeMajor === 23 && nodeMinor >= 5) || (nodeMajor === 22 && nodeMinor >= 15);
12+
13+
// module.register() requires Node >= 20.6.0.
14+
export const supportsRegister = nodeMajor > 20 || (nodeMajor === 20 && nodeMinor >= 6);

scripts/ts-resolve-hooks.js

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,13 @@
66
* - load: for .ts files, delegate to Node's native loader (works on
77
* Node >= 22.6 with --experimental-strip-types). On older Node versions,
88
* throws a clear error instead of returning unparseable TypeScript source.
9+
*
10+
* Each hook is exported twice with identical fallback logic:
11+
* - `resolve`/`load` implement the asynchronous module customization API
12+
* consumed by the deprecated module.register().
13+
* - `resolveSync`/`loadSync` implement the synchronous API consumed by
14+
* module.registerHooks(), whose nextResolve/nextLoad return values (and
15+
* thrown errors) directly rather than via Promises.
916
*/
1017

1118
import { fileURLToPath } from 'node:url';
@@ -27,6 +34,23 @@ export async function resolve(specifier, context, nextResolve) {
2734
}
2835
}
2936

37+
export function resolveSync(specifier, context, nextResolve) {
38+
try {
39+
return nextResolve(specifier, context);
40+
} catch (err) {
41+
// Only intercept ERR_MODULE_NOT_FOUND for .js specifiers
42+
if (err.code === 'ERR_MODULE_NOT_FOUND' && specifier.endsWith('.js')) {
43+
const tsSpecifier = specifier.replace(/\.js$/, '.ts');
44+
try {
45+
return nextResolve(tsSpecifier, context);
46+
} catch {
47+
// .ts also not found — throw the original error
48+
}
49+
}
50+
throw err;
51+
}
52+
}
53+
3054
export async function load(url, context, nextLoad) {
3155
if (!url.endsWith('.ts')) return nextLoad(url, context);
3256

@@ -37,10 +61,27 @@ export async function load(url, context, nextLoad) {
3761
if (err.code !== 'ERR_UNKNOWN_FILE_EXTENSION') throw err;
3862
}
3963

40-
// Node < 22.6 cannot strip TypeScript syntax. Throw a clear error instead
41-
// of returning raw TS source that would produce a confusing SyntaxError.
64+
throw unsupportedTsError(url);
65+
}
66+
67+
export function loadSync(url, context, nextLoad) {
68+
if (!url.endsWith('.ts')) return nextLoad(url, context);
69+
70+
// On Node >= 22.6 with --experimental-strip-types, Node handles .ts natively
71+
try {
72+
return nextLoad(url, context);
73+
} catch (err) {
74+
if (err.code !== 'ERR_UNKNOWN_FILE_EXTENSION') throw err;
75+
}
76+
77+
throw unsupportedTsError(url);
78+
}
79+
80+
// Node < 22.6 cannot strip TypeScript syntax. Throw a clear error instead
81+
// of returning raw TS source that would produce a confusing SyntaxError.
82+
function unsupportedTsError(url) {
4283
const filePath = fileURLToPath(url);
43-
throw Object.assign(
84+
return Object.assign(
4485
new Error(
4586
`Cannot load TypeScript file ${filePath} on Node ${process.versions.node}. ` +
4687
`TypeScript type stripping requires Node >= 22.6 with --experimental-strip-types.`,

scripts/ts-resolve-loader.js

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,22 @@
77
*
88
* Usage: node --import ./scripts/ts-resolve-loader.js ...
99
* (or via NODE_OPTIONS / vitest poolOptions.execArgv)
10+
*
11+
* Prefers module.registerHooks() (synchronous, in-thread, no worker-thread
12+
* overhead) where available, falling back to the deprecated (DEP0205)
13+
* module.register() on older Node versions that predate registerHooks()'s
14+
* availability floor.
1015
*/
1116

12-
// module.register() requires Node >= 20.6.0
13-
const [_major, _minor] = process.versions.node.split('.').map(Number);
14-
if (_major > 20 || (_major === 20 && _minor >= 6)) {
17+
import { supportsRegister, supportsRegisterHooks } from './node-version-support.js';
18+
19+
const hooksURL = new URL('./ts-resolve-hooks.js', import.meta.url);
20+
21+
if (supportsRegisterHooks) {
22+
const { registerHooks } = await import('node:module');
23+
const { resolveSync, loadSync } = await import(hooksURL.href);
24+
registerHooks({ resolve: resolveSync, load: loadSync });
25+
} else if (supportsRegister) {
1526
const { register } = await import('node:module');
16-
const hooksURL = new URL('./ts-resolve-hooks.js', import.meta.url);
1727
register(hooksURL.href, { parentURL: import.meta.url });
1828
}
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
/**
2+
* Regression tests for scripts/ts-resolve-loader.js (issue #1832).
3+
*
4+
* Verifies that:
5+
* - the .js -> .ts fallback resolution still works when loaded via
6+
* `--import` in a spawned child process (functional behavior preserved
7+
* across the module.register() -> module.registerHooks() migration).
8+
* - Node's DEP0205 deprecation warning (module.register()) is no longer
9+
* printed on Node versions that support module.registerHooks().
10+
*/
11+
import { execFileSync, spawnSync } from 'node:child_process';
12+
import fs from 'node:fs';
13+
import os from 'node:os';
14+
import path from 'node:path';
15+
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
16+
import { supportsRegisterHooks } from '../../scripts/node-version-support.js';
17+
18+
const LOADER_URL = new URL('../../scripts/ts-resolve-loader.js', import.meta.url).href;
19+
20+
describe('scripts/ts-resolve-loader.js', () => {
21+
let tmpDir: string;
22+
23+
beforeEach(() => {
24+
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-ts-resolve-loader-'));
25+
});
26+
27+
afterEach(() => {
28+
fs.rmSync(tmpDir, { recursive: true, force: true });
29+
});
30+
31+
it('resolves a .js import specifier to the sibling .ts file', () => {
32+
fs.writeFileSync(
33+
path.join(tmpDir, 'lib.ts'),
34+
'export function greet(): string { return "hello"; }\n',
35+
);
36+
fs.writeFileSync(
37+
path.join(tmpDir, 'main.mjs'),
38+
"import { greet } from './lib.js';\nconsole.log(greet());\n",
39+
);
40+
41+
const stdout = execFileSync(
42+
process.execPath,
43+
['--experimental-strip-types', '--import', LOADER_URL, path.join(tmpDir, 'main.mjs')],
44+
{ encoding: 'utf-8', timeout: 30_000 },
45+
);
46+
47+
expect(stdout.trim()).toBe('hello');
48+
});
49+
50+
it('falls through to the normal ERR_MODULE_NOT_FOUND when neither .js nor .ts exists', () => {
51+
fs.writeFileSync(path.join(tmpDir, 'main.mjs'), "import './does-not-exist.js';\n");
52+
53+
expect(() =>
54+
execFileSync(
55+
process.execPath,
56+
['--experimental-strip-types', '--import', LOADER_URL, path.join(tmpDir, 'main.mjs')],
57+
{ encoding: 'utf-8', timeout: 30_000, stdio: ['ignore', 'pipe', 'pipe'] },
58+
),
59+
).toThrow(/ERR_MODULE_NOT_FOUND/);
60+
});
61+
62+
it.runIf(supportsRegisterHooks)(
63+
'does not print the DEP0205 module.register() deprecation warning',
64+
() => {
65+
fs.writeFileSync(path.join(tmpDir, 'lib.ts'), 'export const x = 1;\n');
66+
fs.writeFileSync(path.join(tmpDir, 'main.mjs'), "import './lib.js';\n");
67+
68+
const result = spawnSync(
69+
process.execPath,
70+
['--experimental-strip-types', '--import', LOADER_URL, path.join(tmpDir, 'main.mjs')],
71+
{ encoding: 'utf-8', timeout: 30_000 },
72+
);
73+
74+
expect(result.status).toBe(0);
75+
expect(result.stderr).not.toMatch(/DEP0205/);
76+
expect(result.stderr).not.toMatch(/module\.register\(\) is deprecated/);
77+
},
78+
);
79+
});

0 commit comments

Comments
 (0)