FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

module: cache synchronous module jobs before linking · nodejs/node@30ed93d · GitHub

/ node Public

Commit 30ed93d

Browse files
joyeecheung
authored and
marco-ippolito
committed
module: cache synchronous module jobs before linking
So that if there are circular dependencies in the synchronous module graph, they could be resolved using the cached jobs. In case linking fails and the error gets caught, reset the cache right after linking. If it succeeds, the caller will cache it again. Otherwise the error bubbles up to users, and since we unset the cache for the unlinkable module the next attempt would still fail. PR-URL: #52868 Backport-PR-URL: #56927 Fixes: #52864 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Refs: #52697
1 parent a03faf2 commit 30ed93d

6 files changed

Lines changed: 43 additions & 14 deletions

File tree

‎lib/internal/modules/esm/module_job.js‎

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -287,22 +287,33 @@ class ModuleJobSync extends ModuleJobBase {
287287
#loader = null;
288288
constructor(loader, url, importAttributes, moduleWrap, isMain, inspectBrk) {
289289
super(url, importAttributes, moduleWrap, isMain, inspectBrk, true);
290-
assert(this.module instanceof ModuleWrap);
291290
this.#loader = loader;
292-
const moduleRequests = this.module.getModuleRequests();
293-
// Specifiers should be aligned with the moduleRequests array in order.
294-
const specifiers = Array(moduleRequests.length);
295-
const modules = Array(moduleRequests.length);
296-
const jobs = Array(moduleRequests.length);
297-
for (let i = 0; i < moduleRequests.length; ++i) {
298-
const { specifier, attributes } = moduleRequests[i];
299-
const job = this.#loader.getModuleJobForRequire(specifier, url, attributes);
300-
specifiers[i] = specifier;
301-
modules[i] = job.module;
302-
jobs[i] = job;
291+
292+
assert(this.module instanceof ModuleWrap);
293+
// Store itself into the cache first before linking in case there are circular
294+
// references in the linking.
295+
loader.loadCache.set(url, importAttributes.type, this);
296+
297+
try {
298+
const moduleRequests = this.module.getModuleRequests();
299+
// Specifiers should be aligned with the moduleRequests array in order.
300+
const specifiers = Array(moduleRequests.length);
301+
const modules = Array(moduleRequests.length);
302+
const jobs = Array(moduleRequests.length);
303+
for (let i = 0; i < moduleRequests.length; ++i) {
304+
const { specifier, attributes } = moduleRequests[i];
305+
const job = this.#loader.getModuleJobForRequire(specifier, url, attributes);
306+
specifiers[i] = specifier;
307+
modules[i] = job.module;
308+
jobs[i] = job;
309+
}
310+
this.module.link(specifiers, modules);
311+
this.linked = jobs;
312+
} finally {
313+
// Restore it - if it succeeds, we'll reset in the caller; Otherwise it's
314+
// not cached and if the error is caught, subsequent attempt would still fail.
315+
loader.loadCache.delete(url, importAttributes.type);
303316
}
304-
this.module.link(specifiers, modules);
305-
this.linked = jobs;
306317
}
307318

308319
get modulePromise() {

‎lib/internal/modules/esm/module_map.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,12 @@ class LoadCache extends SafeMap {
114114
validateString(type, 'type');
115115
return super.get(url)?.[type] !== undefined;
116116
}
117+
delete(url, type = kImplicitAssertType) {
118+
const cached = super.get(url);
119+
if (cached) {
120+
cached[type] = undefined;
121+
}
122+
}
117123
}
118124

119125
module.exports = {

‎test/es-module/test-require-module-cycle-cjs-esm-esm.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// Flags: --experimental-require-module
2+
'use strict';
3+
4+
// This tests that ESM <-> ESM cycle is allowed in a require()-d graph.
5+
const common = require('../common');
6+
const cycle = require('../fixtures/es-modules/cjs-esm-esm-cycle/c.cjs');
7+
8+
common.expectRequiredModule(cycle, { b: 5 });

‎test/fixtures/es-modules/cjs-esm-esm-cycle/a.mjs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export { b } from './b.mjs';

‎test/fixtures/es-modules/cjs-esm-esm-cycle/b.mjs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
import './a.mjs'
2+
export const b = 5;

‎test/fixtures/es-modules/cjs-esm-esm-cycle/c.cjs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
module.exports = require('./a.mjs');

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL