Skip to content

Commit 3b867e4

Browse files
aduh95targos
authored andcommitted
esm: do not give wrong hints when detecting file format
PR-URL: #50314 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
1 parent b61707e commit 3b867e4

File tree

3 files changed

+48
-15
lines changed

3 files changed

+48
-15
lines changed

lib/internal/modules/esm/get_format.js

+11-5
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,10 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE
110110
default: { // The user did not pass `--experimental-default-type`.
111111
// `source` is undefined when this is called from `defaultResolve`;
112112
// but this gets called again from `defaultLoad`/`defaultLoadSync`.
113-
if (source && getOptionValue('--experimental-detect-module')) {
114-
return containsModuleSyntax(`${source}`, fileURLToPath(url)) ? 'module' : 'commonjs';
113+
if (getOptionValue('--experimental-detect-module')) {
114+
return source ?
115+
(containsModuleSyntax(`${source}`, fileURLToPath(url)) ? 'module' : 'commonjs') :
116+
null;
115117
}
116118
return 'commonjs';
117119
}
@@ -136,9 +138,13 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE
136138
return 'commonjs';
137139
}
138140
default: { // The user did not pass `--experimental-default-type`.
139-
if (source && getOptionValue('--experimental-detect-module') &&
140-
getFormatOfExtensionlessFile(url) === 'module') {
141-
return containsModuleSyntax(`${source}`, fileURLToPath(url)) ? 'module' : 'commonjs';
141+
if (getOptionValue('--experimental-detect-module')) {
142+
if (!source) { return null; }
143+
const format = getFormatOfExtensionlessFile(url);
144+
if (format === 'module') {
145+
return containsModuleSyntax(`${source}`, fileURLToPath(url)) ? 'module' : 'commonjs';
146+
}
147+
return format;
142148
}
143149
return 'commonjs';
144150
}

lib/internal/modules/esm/load.js

+14-10
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ function getSourceSync(url, context) {
105105
* @param {LoadContext} context
106106
* @returns {LoadReturn}
107107
*/
108-
async function defaultLoad(url, context = { __proto__: null }) {
108+
async function defaultLoad(url, context = kEmptyObject) {
109109
let responseURL = url;
110110
let {
111111
importAttributes,
@@ -129,18 +129,22 @@ async function defaultLoad(url, context = { __proto__: null }) {
129129

130130
if (urlInstance.protocol === 'node:') {
131131
source = null;
132-
} else if (source == null) {
133-
({ responseURL, source } = await getSource(urlInstance, context));
134-
context.source = source;
135-
}
132+
format ??= 'builtin';
133+
} else {
134+
let contextToPass = context;
135+
if (source == null) {
136+
({ responseURL, source } = await getSource(urlInstance, context));
137+
contextToPass = { __proto__: context, source };
138+
}
136139

137-
if (format == null || format === 'commonjs') {
138140
// Now that we have the source for the module, run `defaultGetFormat` again in case we detect ESM syntax.
139-
format = await defaultGetFormat(urlInstance, context);
140-
}
141+
format ??= await defaultGetFormat(urlInstance, contextToPass);
141142

142-
if (format === 'commonjs') {
143-
source = null; // Let the CommonJS loader handle it (for now)
143+
if (format === 'commonjs' && contextToPass !== context) {
144+
// For backward compatibility reasons, we need to discard the source in
145+
// order for the CJS loader to re-fetch it.
146+
source = null;
147+
}
144148
}
145149

146150
validateAttributes(url, format, importAttributes);

test/es-module/test-esm-detect-ambiguous.mjs

+23
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,29 @@ describe('--experimental-detect-module', { concurrency: true }, () => {
152152
strictEqual(signal, null);
153153
});
154154
}
155+
156+
it('should not hint wrong format in resolve hook', async () => {
157+
let writeSync;
158+
const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
159+
'--experimental-detect-module',
160+
'--no-warnings',
161+
'--loader',
162+
`data:text/javascript,import { writeSync } from "node:fs"; export ${encodeURIComponent(
163+
async function resolve(s, c, next) {
164+
const result = await next(s, c);
165+
writeSync(1, result.format + '\n');
166+
return result;
167+
}
168+
)}`,
169+
fixtures.path('es-modules/package-without-type/noext-esm'),
170+
]);
171+
172+
strictEqual(stderr, '');
173+
strictEqual(stdout, 'null\nexecuted\n');
174+
strictEqual(code, 0);
175+
strictEqual(signal, null);
176+
177+
});
155178
});
156179

157180
describe('file input in a "type": "commonjs" package', { concurrency: true }, () => {

0 commit comments

Comments
 (0)