Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

esm: do not give wrong hints when detecting file format #50314

Merged
merged 2 commits into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions lib/internal/modules/esm/get_format.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,10 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE
default: { // The user did not pass `--experimental-default-type`.
// `source` is undefined when this is called from `defaultResolve`;
// but this gets called again from `defaultLoad`/`defaultLoadSync`.
if (source && getOptionValue('--experimental-detect-module')) {
return containsModuleSyntax(`${source}`, fileURLToPath(url)) ? 'module' : 'commonjs';
if (getOptionValue('--experimental-detect-module')) {
return source ?
(containsModuleSyntax(`${source}`, fileURLToPath(url)) ? 'module' : 'commonjs') :
null;
}
return 'commonjs';
}
Expand All @@ -136,9 +138,13 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE
return 'commonjs';
}
default: { // The user did not pass `--experimental-default-type`.
if (source && getOptionValue('--experimental-detect-module') &&
getFormatOfExtensionlessFile(url) === 'module') {
return containsModuleSyntax(`${source}`, fileURLToPath(url)) ? 'module' : 'commonjs';
if (getOptionValue('--experimental-detect-module')) {
if (!source) { return null; }
const format = getFormatOfExtensionlessFile(url);
if (format === 'module') {
return containsModuleSyntax(`${source}`, fileURLToPath(url)) ? 'module' : 'commonjs';
}
return format;
}
return 'commonjs';
}
Expand Down
24 changes: 14 additions & 10 deletions lib/internal/modules/esm/load.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ function getSourceSync(url, context) {
* @param {LoadContext} context
* @returns {LoadReturn}
*/
async function defaultLoad(url, context = { __proto__: null }) {
async function defaultLoad(url, context = kEmptyObject) {
let responseURL = url;
let {
importAttributes,
Expand All @@ -129,18 +129,22 @@ async function defaultLoad(url, context = { __proto__: null }) {

if (urlInstance.protocol === 'node:') {
source = null;
} else if (source == null) {
({ responseURL, source } = await getSource(urlInstance, context));
context.source = source;
}
format ??= 'builtin';
} else {
let contextToPass = context;
if (source == null) {
({ responseURL, source } = await getSource(urlInstance, context));
contextToPass = { __proto__: context, source };
GeoffreyBooth marked this conversation as resolved.
Show resolved Hide resolved
}

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

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

validateAttributes(url, format, importAttributes);
Expand Down
23 changes: 23 additions & 0 deletions test/es-module/test-esm-detect-ambiguous.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,29 @@ describe('--experimental-detect-module', { concurrency: true }, () => {
strictEqual(signal, null);
});
}

it('should not hint wrong format in resolve hook', async () => {
let writeSync;
const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
'--experimental-detect-module',
'--no-warnings',
'--loader',
`data:text/javascript,import{writeSync}from"node:fs";export ${encodeURIComponent(
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
async function resolve(s, c, next) {
const result = await next(s, c);
writeSync(1, result.format + '\n');
return result;
}
)}`,
fixtures.path('es-modules/package-without-type/noext-esm'),
]);

strictEqual(stderr, '');
strictEqual(stdout, 'null\nexecuted\n');
strictEqual(code, 0);
strictEqual(signal, null);

});
});

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