Skip to content

Commit 1d7b4de

Browse files
committed
review findings part 1. split into two PRs will follow
1 parent 402675a commit 1d7b4de

File tree

2 files changed

+23
-9
lines changed

2 files changed

+23
-9
lines changed

lib/internal/modules/esm/get_format.js

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ const experimentalNetworkImports =
2222
const { containsModuleSyntax } = internalBinding('contextify');
2323
const { getPackageScopeConfig, getPackageType } = require('internal/modules/package_json_reader');
2424
const { fileURLToPath } = require('internal/url');
25+
const { readFileSync } = require('fs');
26+
const { Buffer } = require('buffer');
2527
const { ERR_UNKNOWN_FILE_EXTENSION } = require('internal/errors').codes;
2628

2729
const protocolHandlers = {
@@ -82,6 +84,24 @@ function underNodeModules(url) {
8284
return StringPrototypeIncludes(url.pathname, '/node_modules/');
8385
}
8486

87+
/**
88+
* Determine whether the given source contains CJS or ESM module syntax.
89+
* @param {string} source
90+
* @param {URL} url
91+
*/
92+
function detectModuleFormat(source, url) {
93+
try {
94+
let realSource = source ?? readFileSync(url, 'utf8');
95+
if (Buffer.isBuffer(realSource)) {
96+
// `containsModuleSyntax` requires source to be passed in as string
97+
realSource = realSource.toString();
98+
}
99+
return containsModuleSyntax(realSource, fileURLToPath(url), url) ? 'module' : 'commonjs';
100+
} catch {
101+
return 'commonjs';
102+
}
103+
}
104+
85105
let typelessPackageJsonFilesWarnedAbout;
86106
/**
87107
* @param {URL} url
@@ -92,12 +112,6 @@ let typelessPackageJsonFilesWarnedAbout;
92112
function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreErrors) {
93113
const { source } = context;
94114
const ext = extname(url);
95-
const deduceFormat = (fromSource, fromUrl) => {
96-
const { existsSync, readFileSync } = require('fs');
97-
const realSource = fromSource ?? existsSync(fromUrl) ? readFileSync(fromUrl).toString() : undefined;
98-
return realSource ? // Do we have a source? check for module syntax
99-
(containsModuleSyntax(realSource, fileURLToPath(fromUrl), fromUrl) ? 'module' : 'commonjs') : 'commonjs';
100-
};
101115

102116
if (ext === '.js') {
103117
const { type: packageType, pjsonPath } = getPackageScopeConfig(url);
@@ -119,7 +133,7 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE
119133
// `source` is undefined when this is called from `defaultResolve`;
120134
// but this gets called again from `defaultLoad`/`defaultLoadSync`.
121135
if (getOptionValue('--experimental-detect-module')) {
122-
const format = deduceFormat(source, url);
136+
const format = detectModuleFormat(source, url);
123137
if (format === 'module') {
124138
// This module has a .js extension, a package.json with no `type` field, and ESM syntax.
125139
// Warn about the missing `type` field so that the user can avoid the performance penalty of detection.
@@ -160,7 +174,7 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE
160174
default: { // The user did not pass `--experimental-default-type`.
161175
if (getOptionValue('--experimental-detect-module')) {
162176
const format = getFormatOfExtensionlessFile(url);
163-
return (format === 'module') ? deduceFormat(source, url) : format;
177+
return (format === 'module') ? detectModuleFormat(source, url) : format;
164178
}
165179
return 'commonjs';
166180
}

test/es-module/test-esm-cjs-exports.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,6 @@ describe('ESM: importing CJS', { concurrency: !process.env.TEST_PARALLEL }, () =
2828
assert.strictEqual(code, 0);
2929
assert.strictEqual(signal, null);
3030
assert.strictEqual(stdout, '');
31-
assert.match(stderr, /[MODULE_TYPELESS_PACKAGE_JSON]/);
31+
assert.match(stderr, /\[MODULE_TYPELESS_PACKAGE_JSON\]/);
3232
});
3333
});

0 commit comments

Comments
 (0)