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

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

/ node Public

Commit f74beb5

Browse files
joyeecheung
authored and
targos
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 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>
1 parent 1f8eaec commit f74beb5

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
@@ -295,22 +295,33 @@ class ModuleJobSync extends ModuleJobBase {
295295
#loader = null;
296296
constructor(loader, url, importAttributes, moduleWrap, isMain, inspectBrk) {
297297
super(url, importAttributes, moduleWrap, isMain, inspectBrk, true);
298-
assert(this.module instanceof ModuleWrap);
299298
this.#loader = loader;
300-
const moduleRequests = this.module.getModuleRequests();
301-
// Specifiers should be aligned with the moduleRequests array in order.
302-
const specifiers = Array(moduleRequests.length);
303-
const modules = Array(moduleRequests.length);
304-
const jobs = Array(moduleRequests.length);
305-
for (let i = 0; i < moduleRequests.length; ++i) {
306-
const { specifier, attributes } = moduleRequests[i];
307-
const job = this.#loader.getModuleJobForRequire(specifier, url, attributes);
308-
specifiers[i] = specifier;
309-
modules[i] = job.module;
310-
jobs[i] = job;
299+
300+
assert(this.module instanceof ModuleWrap);
301+
// Store itself into the cache first before linking in case there are circular
302+
// references in the linking.
303+
loader.loadCache.set(url, importAttributes.type, this);
304+
305+
try {
306+
const moduleRequests = this.module.getModuleRequests();
307+
// Specifiers should be aligned with the moduleRequests array in order.
308+
const specifiers = Array(moduleRequests.length);
309+
const modules = Array(moduleRequests.length);
310+
const jobs = Array(moduleRequests.length);
311+
for (let i = 0; i < moduleRequests.length; ++i) {
312+
const { specifier, attributes } = moduleRequests[i];
313+
const job = this.#loader.getModuleJobForRequire(specifier, url, attributes);
314+
specifiers[i] = specifier;
315+
modules[i] = job.module;
316+
jobs[i] = job;
317+
}
318+
this.module.link(specifiers, modules);
319+
this.linked = jobs;
320+
} finally {
321+
// Restore it - if it succeeds, we'll reset in the caller; Otherwise it's
322+
// not cached and if the error is caught, subsequent attempt would still fail.
323+
loader.loadCache.delete(url, importAttributes.type);
311324
}
312-
this.module.link(specifiers, modules);
313-
this.linked = jobs;
314325
}
315326

316327
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