Skip to content

Commit 8e780bc

Browse files
joyeecheungaduh95
authored andcommitted
module: use synchronous hooks for preparsing in import(cjs)
PR-URL: #55698 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Guy Bedford <[email protected]>
1 parent e5bb6c2 commit 8e780bc

File tree

1 file changed

+34
-31
lines changed

1 file changed

+34
-31
lines changed

lib/internal/modules/esm/translators.js

+34-31
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const {
2626
const { BuiltinModule } = require('internal/bootstrap/realm');
2727
const assert = require('internal/assert');
2828
const { readFileSync } = require('fs');
29-
const { dirname, extname, isAbsolute } = require('path');
29+
const { dirname, extname } = require('path');
3030
const {
3131
assertBufferSource,
3232
loadBuiltinModule,
@@ -42,6 +42,9 @@ const {
4242
kModuleSource,
4343
kModuleExport,
4444
kModuleExportNames,
45+
findLongestRegisteredExtension,
46+
resolveForCJSWithHooks,
47+
loadSourceForCJSWithHooks,
4548
} = require('internal/modules/cjs/loader');
4649
const { fileURLToPath, pathToFileURL, URL } = require('internal/url');
4750
let debug = require('internal/util/debuglog').debuglog('esm', (fn) => {
@@ -171,17 +174,18 @@ const cjsCache = new SafeMap();
171174
* @param {string} url - The URL of the module.
172175
* @param {string} source - The source code of the module.
173176
* @param {boolean} isMain - Whether the module is the main module.
177+
* @param {string} format - Format of the module.
174178
* @param {typeof loadCJSModule} [loadCJS=loadCJSModule] - The function to load the CommonJS module.
175179
* @returns {ModuleWrap} The ModuleWrap object for the CommonJS module.
176180
*/
177-
function createCJSModuleWrap(url, source, isMain, loadCJS = loadCJSModule) {
181+
function createCJSModuleWrap(url, source, isMain, format, loadCJS = loadCJSModule) {
178182
debug(`Translating CJSModule ${url}`);
179183

180184
const filename = urlToFilename(url);
181185
// In case the source was not provided by the `load` step, we need fetch it now.
182186
source = stringify(source ?? getSource(new URL(url)).source);
183187

184-
const { exportNames, module } = cjsPreparseModuleExports(filename, source);
188+
const { exportNames, module } = cjsPreparseModuleExports(filename, source, isMain, format);
185189
cjsCache.set(url, module);
186190

187191
const wrapperNames = [...exportNames, 'module.exports'];
@@ -228,7 +232,7 @@ function createCJSModuleWrap(url, source, isMain, loadCJS = loadCJSModule) {
228232
translators.set('commonjs-sync', function requireCommonJS(url, source, isMain) {
229233
initCJSParseSync();
230234

231-
return createCJSModuleWrap(url, source, isMain, (module, source, url, filename, isMain) => {
235+
return createCJSModuleWrap(url, source, isMain, 'commonjs', (module, source, url, filename, isMain) => {
232236
assert(module === CJSModule._cache[filename]);
233237
wrapModuleLoad(filename, null, isMain);
234238
});
@@ -240,7 +244,7 @@ translators.set('require-commonjs', (url, source, isMain) => {
240244
initCJSParseSync();
241245
assert(cjsParse);
242246

243-
return createCJSModuleWrap(url, source);
247+
return createCJSModuleWrap(url, source, isMain, 'commonjs');
244248
});
245249

246250
// Handle CommonJS modules referenced by `require` calls.
@@ -249,7 +253,7 @@ translators.set('require-commonjs-typescript', (url, source, isMain) => {
249253
emitExperimentalWarning('Type Stripping');
250254
assert(cjsParse);
251255
const code = stripTypeScriptModuleTypes(stringify(source), url);
252-
return createCJSModuleWrap(url, code);
256+
return createCJSModuleWrap(url, code, isMain, 'commonjs-typescript');
253257
});
254258

255259
// Handle CommonJS modules referenced by `import` statements or expressions,
@@ -273,16 +277,17 @@ translators.set('commonjs', function commonjsStrategy(url, source, isMain) {
273277
} catch {
274278
// Continue regardless of error.
275279
}
276-
return createCJSModuleWrap(url, source, isMain, cjsLoader);
280+
return createCJSModuleWrap(url, source, isMain, 'commonjs', cjsLoader);
277281
});
278282

279283
/**
280284
* Pre-parses a CommonJS module's exports and re-exports.
281285
* @param {string} filename - The filename of the module.
282286
* @param {string} [source] - The source code of the module.
287+
* @param {boolean} isMain - Whether it is pre-parsing for the entry point.
288+
* @param {string} format
283289
*/
284-
function cjsPreparseModuleExports(filename, source) {
285-
// TODO: Do we want to keep hitting the user mutable CJS loader here?
290+
function cjsPreparseModuleExports(filename, source, isMain, format) {
286291
let module = CJSModule._cache[filename];
287292
if (module && module[kModuleExportNames] !== undefined) {
288293
return { module, exportNames: module[kModuleExportNames] };
@@ -293,10 +298,15 @@ function cjsPreparseModuleExports(filename, source) {
293298
module.filename = filename;
294299
module.paths = CJSModule._nodeModulePaths(module.path);
295300
module[kIsCachedByESMLoader] = true;
296-
module[kModuleSource] = source;
297301
CJSModule._cache[filename] = module;
298302
}
299303

304+
if (source === undefined) {
305+
({ source } = loadSourceForCJSWithHooks(module, filename, format));
306+
}
307+
module[kModuleSource] = source;
308+
309+
debug(`Preparsing exports of ${filename}`);
300310
let exports, reexports;
301311
try {
302312
({ exports, reexports } = cjsParse(source || ''));
@@ -310,34 +320,27 @@ function cjsPreparseModuleExports(filename, source) {
310320
// Set first for cycles.
311321
module[kModuleExportNames] = exportNames;
312322

323+
// If there are any re-exports e.g. `module.exports = { ...require(...) }`,
324+
// pre-parse the dependencies to find transitively exported names.
313325
if (reexports.length) {
314-
module.filename = filename;
315-
module.paths = CJSModule._nodeModulePaths(module.path);
326+
module.filename ??= filename;
327+
module.paths ??= CJSModule._nodeModulePaths(dirname(filename));
328+
316329
for (let i = 0; i < reexports.length; i++) {
330+
debug(`Preparsing re-exports of '${filename}'`);
317331
const reexport = reexports[i];
318332
let resolved;
333+
let format;
319334
try {
320-
// TODO: this should be calling the `resolve` hook chain instead.
321-
// Doing so would mean dropping support for CJS in the loader thread, as
322-
// this call needs to be sync from the perspective of the main thread,
323-
// which we can do via HooksProxy and Atomics, but we can't do within
324-
// the loaders thread. Until this is done, the lexer will use the
325-
// monkey-patchable CJS loader to get the path to the module file to
326-
// load (which may or may not be aligned with the URL that the `resolve`
327-
// hook have returned).
328-
resolved = CJSModule._resolveFilename(reexport, module);
329-
} catch {
335+
({ format, filename: resolved } = resolveForCJSWithHooks(reexport, module, false));
336+
} catch (e) {
337+
debug(`Failed to resolve '${reexport}', skipping`, e);
330338
continue;
331339
}
332-
// TODO: this should be calling the `load` hook chain and check if it returns
333-
// `format: 'commonjs'` instead of relying on file extensions.
334-
const ext = extname(resolved);
335-
if ((ext === '.js' || ext === '.cjs' || !CJSModule._extensions[ext]) &&
336-
isAbsolute(resolved)) {
337-
// TODO: this should be calling the `load` hook chain to get the source
338-
// (and fallback to reading the FS only if the source is nullish).
339-
const source = readFileSync(resolved, 'utf-8');
340-
const { exportNames: reexportNames } = cjsPreparseModuleExports(resolved, source);
340+
341+
if (format === 'commonjs' ||
342+
(!BuiltinModule.normalizeRequirableId(resolved) && findLongestRegisteredExtension(resolved) === '.js')) {
343+
const { exportNames: reexportNames } = cjsPreparseModuleExports(resolved, undefined, false, format);
341344
for (const name of reexportNames) {
342345
exportNames.add(name);
343346
}

0 commit comments

Comments
 (0)