Skip to content

Commit 1ce8463

Browse files
fix: resolve CommonJS directory requires inside dependencies (#803)
Co-authored-by: Hiroki Osame <hiroki.osame@gmail.com>
1 parent dce02fc commit 1ce8463

2 files changed

Lines changed: 168 additions & 12 deletions

File tree

src/esm/hook/resolve.ts

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import path from 'node:path';
2-
import { pathToFileURL } from 'node:url';
2+
import { fileURLToPath, pathToFileURL } from 'node:url';
33
import type {
44
ResolveHook,
55
ResolveHookContext,
@@ -96,6 +96,13 @@ const isModuleNotFound = (
9696
|| code === 'MODULE_NOT_FOUND'
9797
);
9898

99+
const isCommonJsRequireContext = (
100+
context: ResolveHookContext,
101+
) => (
102+
context.conditions.includes('require')
103+
&& !context.conditions.includes('import')
104+
);
105+
99106
const resolveExtensions = async (
100107
url: string,
101108
context: ResolveHookContext,
@@ -387,17 +394,38 @@ const resolveDirectorySync = (
387394
}
388395

389396
if (isDirectoryPattern.test(specifier)) {
397+
// On Node's sync hooks, a CommonJS require() inside a dependency reaches
398+
// this hook. A bare specifier with a trailing slash (e.g. `process/`) is a
399+
// package, not a relative directory, so defer to resolveBaseSync, which
400+
// lets Node resolve the package while retrying TypeScript extensions.
401+
// https://github.com/privatenumber/tsx/issues/800
402+
const isCjsRequire = isCommonJsRequireContext(context);
403+
if (isCjsRequire && !isFilePath(specifier)) {
404+
return resolveBaseSync(specifier, context, nextResolve, hookData);
405+
}
406+
390407
const urlParsed = new URL(specifier, context.parentURL);
391408

392409
// If directory, can be index.js, index.ts, etc.
393410
urlParsed.pathname = path.join(urlParsed.pathname, 'index');
394411

395-
return resolveExtensionsSync(
396-
urlParsed.toString(),
412+
if (!isCjsRequire) {
413+
return resolveExtensionsSync(urlParsed.toString(), context, nextResolve, true)!;
414+
}
415+
416+
// Node's CommonJS resolver rejects file:// URLs, so resolve the implicit
417+
// index from a filesystem path. Fall back to Node's directory resolution
418+
// (package.json "main") via resolveBaseSync when no index file exists.
419+
//
420+
// This prefers the index over "main", matching tsx's CommonJS loader
421+
// (which prioritizes index.ts). Native Node resolves "main" first.
422+
const indexResolved = resolveExtensionsSync(
423+
fileURLToPath(urlParsed),
397424
context,
398425
nextResolve,
399-
true,
400-
)!;
426+
false,
427+
);
428+
return indexResolved ?? resolveBaseSync(specifier, context, nextResolve, hookData);
401429
}
402430

403431
try {
@@ -519,13 +547,6 @@ const resolveTsPathsSync = (
519547

520548
const tsxProtocol = 'tsx://';
521549

522-
const isCommonJsRequireContext = (
523-
context: ResolveHookContext,
524-
) => (
525-
context.conditions.includes('require')
526-
&& !context.conditions.includes('import')
527-
);
528-
529550
const addQuery = (
530551
url: string,
531552
query: string,

tests/specs/version-sensitive.ts

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,141 @@ export const versionSensitiveTests = (node: NodeApis) => describe('Version-sensi
314314
});
315315
});
316316

317+
// https://github.com/privatenumber/tsx/issues/800
318+
test('sync ESM hook resolves directory requires inside dependencies', async () => {
319+
await using fixture = await createFixture({
320+
'package.json': createPackageJson({ type: 'commonjs' }),
321+
'entry.cjs': 'console.log(JSON.stringify(require("dep")));',
322+
node_modules: {
323+
// Bare dependency required with a trailing slash, like
324+
// readable-stream's `require('process/')`. The trailing slash
325+
// must not be treated as a relative directory.
326+
'bare-dep': {
327+
'package.json': createPackageJson({
328+
type: 'commonjs',
329+
main: './index.js',
330+
}),
331+
'index.js': 'module.exports = "bare-ok";',
332+
},
333+
dep: {
334+
'package.json': createPackageJson({
335+
type: 'commonjs',
336+
main: './lib/sub/entry.js',
337+
}),
338+
lib: {
339+
// `require('..')` from the nested entry resolves here.
340+
'index.js': 'module.exports = { parent: "parent-ok" };',
341+
sub: {
342+
'entry.js': `
343+
const parent = require('..');
344+
const bare = require('bare-dep/');
345+
module.exports = { parent: parent.parent, bare };
346+
`,
347+
},
348+
},
349+
},
350+
},
351+
});
352+
353+
const process = await node.hook(['entry.cjs'], fixture.path);
354+
355+
expect(process.stderr).toBe('');
356+
expect(process.exitCode).toBe(0);
357+
expect(JSON.parse(process.stdout)).toEqual({
358+
parent: 'parent-ok',
359+
bare: 'bare-ok',
360+
});
361+
});
362+
363+
// https://github.com/privatenumber/tsx/issues/800
364+
test('sync ESM hook resolves a dependency directory require to a TypeScript index', async () => {
365+
await using fixture = await createFixture({
366+
'package.json': createPackageJson({ type: 'commonjs' }),
367+
'entry.cjs': 'console.log(require("ts-dep").tag);',
368+
'node_modules/ts-dep': {
369+
'package.json': createPackageJson({
370+
type: 'commonjs',
371+
main: './lib/sub/entry.js',
372+
}),
373+
lib: {
374+
// TypeScript directory index, with no index.js sibling: the
375+
// type annotation proves it is transformed, not found as JS.
376+
'index.ts': 'export const tag: string = "ts-index";',
377+
sub: {
378+
'entry.js': 'module.exports = require("..");',
379+
},
380+
},
381+
},
382+
});
383+
384+
const process = await node.hook(['entry.cjs'], fixture.path);
385+
386+
expect(process.stderr).toBe('');
387+
expect(process.exitCode).toBe(0);
388+
expect(process.stdout).toBe('ts-index');
389+
});
390+
391+
// https://github.com/privatenumber/tsx/issues/800
392+
test('sync ESM hook resolves a dependency directory require via package.json "main"', async () => {
393+
await using fixture = await createFixture({
394+
'package.json': createPackageJson({ type: 'commonjs' }),
395+
'entry.cjs': 'console.log(require("main-dep").from);',
396+
'node_modules/main-dep': {
397+
'package.json': createPackageJson({
398+
type: 'commonjs',
399+
main: './lib/sub/entry.js',
400+
}),
401+
lib: {
402+
// Nested package.json "main" with no index file: require('..')
403+
// must fall back to it rather than a synthesized index.
404+
'package.json': createPackageJson({ main: './real-main.js' }),
405+
'real-main.js': 'module.exports = { from: "real-main" };',
406+
sub: {
407+
'entry.js': 'module.exports = require("..");',
408+
},
409+
},
410+
},
411+
});
412+
413+
const process = await node.hook(['entry.cjs'], fixture.path);
414+
415+
expect(process.stderr).toBe('');
416+
expect(process.exitCode).toBe(0);
417+
expect(process.stdout).toBe('real-main');
418+
});
419+
420+
// https://github.com/privatenumber/tsx/issues/800
421+
test('sync ESM hook resolves a trailing-slash package require to a TypeScript main', async () => {
422+
await using fixture = await createFixture({
423+
'package.json': createPackageJson({ type: 'commonjs' }),
424+
'entry.cjs': 'console.log(require("driver-dep").v);',
425+
node_modules: {
426+
'driver-dep': {
427+
'package.json': createPackageJson({
428+
type: 'commonjs',
429+
main: './entry.js',
430+
}),
431+
// Trailing-slash require of a package whose main is TS-only:
432+
// must still retry TypeScript extensions on the package main.
433+
'entry.js': 'module.exports = require("ts-main-pkg/");',
434+
},
435+
'ts-main-pkg': {
436+
'package.json': createPackageJson({
437+
type: 'commonjs',
438+
main: './main.js',
439+
}),
440+
'main.ts': 'export const v: string = "ts-main";',
441+
},
442+
},
443+
});
444+
445+
const process = await node.hook(['entry.cjs'], fixture.path);
446+
447+
expect(process.stderr).toBe('');
448+
expect(process.exitCode).toBe(0);
449+
expect(process.stdout).toBe('ts-main');
450+
});
451+
317452
await test('watch reruns when imported TypeScript file changes', async () => {
318453
await using fixture = await createFixture({
319454
'package.json': createPackageJson({ type: 'commonjs' }),

0 commit comments

Comments
 (0)