Skip to content

Commit

Permalink
esm: do not give wrong hints when detecting file format
Browse files Browse the repository at this point in the history
PR-URL: #50314
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
  • Loading branch information
aduh95 authored and targos committed Nov 11, 2023
1 parent b61707e commit 3b867e4
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 15 deletions.
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 };
}

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(
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

0 comments on commit 3b867e4

Please sign in to comment.