Skip to content

Commit c188268

Browse files
privatenumberprivatenumber
authored
fix: support ambiguous package (#79)
1 parent 602f1b1 commit c188268

File tree

8 files changed

+93
-98
lines changed

8 files changed

+93
-98
lines changed

‎src/esm/hook/load.ts

Lines changed: 68 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -195,82 +195,86 @@ let load: LoadHook = async (
195195
loaded,
196196
});
197197

198-
if (
199-
isFeatureSupported(loadReadFromSource)
200-
&& loaded.format === 'commonjs'
201-
&& filePath.endsWith('.cjs')
202-
) {
203-
let code = await readFile(new URL(url), 'utf8');
204-
// Contains native ESM check
205-
const transformed = transformDynamicImport(filePath, code);
206-
if (transformed) {
207-
code = inlineSourceMap(transformed);
198+
if (loaded.format === 'commonjs') {
199+
if (
200+
isFeatureSupported(loadReadFromSource)
201+
&& filePath.endsWith('.cjs')
202+
) {
203+
let code = await readFile(new URL(url), 'utf8');
204+
// Contains native ESM check
205+
const transformed = transformDynamicImport(filePath, code);
206+
if (transformed) {
207+
code = inlineSourceMap(transformed);
208+
}
209+
loaded.source = code;
210+
loaded.shortCircuit = true;
211+
return loaded;
208212
}
209-
loaded.source = code;
210-
loaded.shortCircuit = true;
211-
return loaded;
212-
}
213-
214-
if (
215-
loaded.format === 'commonjs'
216-
&& isFeatureSupported(esmLoadReadFile)
217-
&& loaded.responseURL?.startsWith('file:') // Could be data:
218-
&& !filePath.endsWith('.cjs') // CJS syntax doesn't need to be transformed for interop
219-
) {
220-
const code = await readFile(new URL(url), 'utf8');
221213

222-
// if the file extension is .js, only transform if using esm syntax
223214
if (
224-
// TypeScript files
225-
!filePath.endsWith('.js')
226-
227-
// ESM syntax in CommonJS type package
228-
|| isESM(code)
215+
isFeatureSupported(esmLoadReadFile)
216+
&& loaded.responseURL?.startsWith('file:') // Could be data:
217+
&& !filePath.endsWith('.cjs') // CJS syntax doesn't need to be transformed for interop
229218
) {
230-
/**
231-
* es or cjs module lexer unfortunately cannot be used because it doesn't support
232-
* typescript syntax
233-
*
234-
* While the full code is transformed, only the exports are used for parsing.
235-
* In fact, the code can't even run because imports cannot be resolved relative
236-
* from the data: URL.
237-
*
238-
* This should pre-compile for the CJS loader to have a cache hit
239-
*
240-
* I considered extracting the CJS exports from esbuild via (0&&(module.exports={})
241-
* to minimize the data URL size but this only works for ESM->CJS and not CTS files
242-
* which are already in CJS syntax.
243-
* In CTS, module.exports can be written in any pattern.
244-
*/
245-
const transformed = transformSync(
246-
code,
247-
url,
248-
{
249-
tsconfigRaw: fileMatcher?.(filePath) as TransformOptions['tsconfigRaw'],
250-
},
251-
);
219+
const code = await readFile(new URL(url), 'utf8');
252220

253-
if (isFeatureSupported(loadReadFromSource)) {
254-
/**
255-
* Compile ESM to CJS
256-
* In v22.15, the CJS loader logic is now moved to the ESM loader
257-
*/
258-
loaded.source = inlineSourceMap(transformed);
259-
} else {
221+
// if the file extension is .js, only transform if using esm syntax
222+
if (
223+
// TypeScript files
224+
!filePath.endsWith('.js')
225+
226+
// ESM syntax in CommonJS type package
227+
|| isESM(code)
228+
) {
260229
/**
261-
* This tricks Node into thinking the file is a data URL so it doesn't try to read from disk
262-
* to parse the CJS exports
230+
* es or cjs module lexer unfortunately cannot be used because it doesn't support
231+
* typescript syntax
232+
*
233+
* While the full code is transformed, only the exports are used for parsing.
234+
* In fact, the code can't even run because imports cannot be resolved relative
235+
* from the data: URL.
236+
*
237+
* This should pre-compile for the CJS loader to have a cache hit
238+
*
239+
* I considered extracting the CJS exports from esbuild via (0&&(module.exports={})
240+
* to minimize the data URL size but this only works for ESM->CJS and not CTS files
241+
* which are already in CJS syntax.
242+
* In CTS, module.exports can be written in any pattern.
263243
*/
264-
const filePathWithNamespace = urlNamespace ? `${filePath}?namespace=${encodeURIComponent(urlNamespace)}` : filePath;
265-
loaded.responseURL = `data:text/javascript,${encodeURIComponent(transformed.code)}?filePath=${encodeURIComponent(filePathWithNamespace)}`;
244+
const transformed = transformSync(
245+
code,
246+
url,
247+
{
248+
tsconfigRaw: fileMatcher?.(filePath) as TransformOptions['tsconfigRaw'],
249+
},
250+
);
251+
252+
if (isFeatureSupported(loadReadFromSource)) {
253+
/**
254+
* Compile ESM to CJS
255+
* In v22.15, the CJS loader logic is now moved to the ESM loader
256+
*/
257+
loaded.source = inlineSourceMap(transformed);
258+
} else {
259+
/**
260+
* This tricks Node into thinking the file is a data URL so it doesn't try to
261+
* read from disk to parse the CJS exports
262+
*/
263+
const filePathWithNamespace = urlNamespace ? `${filePath}?namespace=${encodeURIComponent(urlNamespace)}` : filePath;
264+
loaded.responseURL = `data:text/javascript,${encodeURIComponent(transformed.code)}?filePath=${encodeURIComponent(filePathWithNamespace)}`;
265+
}
266+
267+
return loaded;
266268
}
267269

268-
log(3, 'returning CJS export annotation', loaded);
269-
return loaded;
270+
if (!loaded.source) {
271+
loaded.source = code;
272+
return loaded;
273+
}
270274
}
271275
}
272276

273-
// CommonJS and Internal modules (e.g. node:*)
277+
// Internal modules (e.g. node:*)
274278
if (!loaded.source) {
275279
return loaded;
276280
}

‎src/esm/hook/resolve.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,10 @@ let resolve: ResolveHook = async (
348348
nextResolve,
349349
);
350350

351+
log(2, 'nextResolve', {
352+
resolved,
353+
});
354+
351355
if (resolved.format === 'builtin') {
352356
return resolved;
353357
}
@@ -359,6 +363,10 @@ let resolve: ResolveHook = async (
359363
&& resolved.url.startsWith(fileUrlPrefix)
360364
) {
361365
resolved.format = await getFormatFromFileUrl(resolved.url);
366+
log(2, 'getFormatFromFileUrl', {
367+
resolved,
368+
format: resolved.format,
369+
});
362370
}
363371

364372
if (query) {

‎src/esm/hook/utils.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import path from 'node:path';
22
import type { ModuleFormat } from 'node:module';
3-
import { tsExtensionsPattern } from '../../utils/path-utils.js';
3+
import { tsExtensions } from '../../utils/path-utils.js';
44
import { getPackageType } from './package-json.js';
55

66
const getFormatFromExtension = (fileUrl: string): ModuleFormat | undefined => {
@@ -24,7 +24,9 @@ export const getFormatFromFileUrl = (fileUrl: string) => {
2424
}
2525

2626
// ts, tsx, jsx
27-
if (tsExtensionsPattern.test(fileUrl)) {
27+
const { pathname } = new URL(fileUrl);
28+
const extension = path.extname(pathname);
29+
if (extension === '.js' || tsExtensions.includes(extension)) {
2830
return getPackageType(fileUrl);
2931
}
3032
};

‎src/utils/path-utils.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ export const requestAcceptsQuery = (request: string) => {
4646

4747
export const fileUrlPrefix = 'file://';
4848

49+
export const tsExtensions = ['.ts', '.tsx', '.jsx', '.mts', '.cts'];
50+
4951
export const tsExtensionsPattern = /\.([cm]?ts|[tj]sx)($|\?)/;
5052

5153
export const cjsExtensionPattern = /[/\\].+\.(?:cts|cjs)(?:$|\?)/;

‎tests/fixtures.ts

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -294,27 +294,6 @@ export const files = {
294294
});
295295
`,
296296
},
297-
// TODO: Package with no type field but ESM syntax
298-
// (also check this in the app itself, not just in node_modules)
299-
'pkg-ambiguous': {
300-
'package.json': createPackageJson({
301-
type: 'commonjs',
302-
main: './index.js',
303-
}),
304-
'index.ts': 'throw new Error("should prefer .js over .ts in node_modules")',
305-
'index.js': syntaxLowering,
306-
'ts.ts': syntaxLowering,
307-
'cjs.js': `
308-
const _ = exports;
309-
const cjsJs = true;
310-
_.cjsJs = cjsJs;
311-
312-
// Annotate CommonJS exports for Node
313-
0 && (module.exports = {
314-
cjsJs,
315-
});
316-
`,
317-
},
318297
'pkg-module': {
319298
'package.json': createPackageJson({
320299
type: 'module',

‎tests/specs/smoke.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,12 @@ const wasmPathUrl = pathToFileURL(wasmPath).toString();
1515
export default testSuite(async ({ describe }, { tsx, supports, version }: NodeApis) => {
1616
describe('Smoke', ({ describe, test }) => {
1717
for (const packageType of packageTypes) {
18-
const isCommonJs = packageType === 'commonjs';
18+
const isCommonJs = packageType !== 'module';
1919

20-
describe(packageType, ({ test }) => {
20+
describe(`package type: ${packageType ?? 'undefined'}`, ({ test }) => {
2121
test('from .js', async ({ onTestFail }) => {
2222
await using fixture = await createFixture({
23-
'package.json': createPackageJson({ type: packageType }),
23+
'package.json': createPackageJson(packageType ? { type: packageType } : {}),
2424
'import-from-js.js': outdent`
2525
import assert from 'assert';
2626
import { expectErrors } from 'expect-errors';
@@ -200,7 +200,7 @@ export default testSuite(async ({ describe }, { tsx, supports, version }: NodeAp
200200

201201
test('from .ts', async ({ onTestFail }) => {
202202
await using fixture = await createFixture({
203-
'package.json': createPackageJson({ type: packageType }),
203+
'package.json': createPackageJson(packageType ? { type: packageType } : {}),
204204

205205
'import-from-ts.ts': ({ fixturePath }) => outdent`
206206
import assert from 'assert';
@@ -464,7 +464,7 @@ export default testSuite(async ({ describe }, { tsx, supports, version }: NodeAp
464464

465465
test('resolve ts in exports', async () => {
466466
await using fixture = await createFixture({
467-
'package.json': createPackageJson({ type: packageType }),
467+
'package.json': createPackageJson(packageType ? { type: packageType } : {}),
468468
'index.ts': `
469469
import A from 'pkg'
470470
console.log(A satisfies 2)
@@ -493,7 +493,7 @@ export default testSuite(async ({ describe }, { tsx, supports, version }: NodeAp
493493
if (!version.startsWith('18.')) {
494494
test('resolve ts in main', async ({ onTestFail }) => {
495495
await using fixture = await createFixture({
496-
'package.json': createPackageJson({ type: packageType }),
496+
'package.json': createPackageJson(packageType ? { type: packageType } : {}),
497497
'index.ts': `
498498
import A from 'pkg'
499499
console.log(A satisfies 2);

‎tests/specs/tsconfig.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ import { packageTypes } from '../utils/package-types.js';
99
export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
1010
describe('tsconfig', ({ describe }) => {
1111
for (const packageType of packageTypes) {
12-
describe(packageType, async ({ describe, onFinish }) => {
12+
describe(`package type: ${packageType ?? 'undefined'}`, async ({ describe, onFinish }) => {
1313
const fixture = await createFixture({
1414
...expectErrors,
1515

16-
'package.json': createPackageJson({ type: packageType }),
16+
'package.json': createPackageJson(packageType ? { type: packageType } : {}),
1717

1818
'tsconfig.json': createTsconfig({
1919
compilerOptions: {
@@ -115,7 +115,7 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
115115
describe('detected tsconfig', ({ test }) => {
116116
test('invalid tsconfig should be ignored', async () => {
117117
await using fixture = await createFixture({
118-
'package.json': createPackageJson({ type: packageType }),
118+
'package.json': createPackageJson(packageType ? { type: packageType } : {}),
119119
'tsconfig.json': createTsconfig({
120120
extends: 'doesnt-exist',
121121
}),
@@ -141,7 +141,7 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
141141
describe('custom tsconfig', ({ test }) => {
142142
test('invalid tsconfig should error', async () => {
143143
await using fixture = await createFixture({
144-
'package.json': createPackageJson({ type: packageType }),
144+
'package.json': createPackageJson(packageType ? { type: packageType } : {}),
145145
'tsconfig.json': createTsconfig({
146146
extends: 'doesnt-exist',
147147
}),

‎tests/utils/package-types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
export const packageTypes = [
2-
// undefined, // TODO: Add ambiguous package type
2+
undefined,
33
'module',
44
'commonjs',
55
] as const;

0 commit comments

Comments
 (0)