Skip to content

Commit 28f0693

Browse files
bnoordhuisaddaleax
authored andcommitted
lib: include cached modules in module.children
`module.children` is supposed to be the list of modules included by this module but lib/module.js failed to update the list when the included module was retrieved from `Module._cache`. Fixes: #7131 PR-URL: #14132 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
1 parent 552d2be commit 28f0693

File tree

5 files changed

+31
-4
lines changed

5 files changed

+31
-4
lines changed

lib/module.js

+8-4
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,17 @@ function stat(filename) {
4646
}
4747
stat.cache = null;
4848

49+
function updateChildren(parent, child, scan) {
50+
var children = parent && parent.children;
51+
if (children && !(scan && children.includes(child)))
52+
children.push(child);
53+
}
4954

5055
function Module(id, parent) {
5156
this.id = id;
5257
this.exports = {};
5358
this.parent = parent;
54-
if (parent && parent.children) {
55-
parent.children.push(this);
56-
}
57-
59+
updateChildren(parent, this, false);
5860
this.filename = null;
5961
this.loaded = false;
6062
this.children = [];
@@ -438,6 +440,7 @@ Module._load = function(request, parent, isMain) {
438440

439441
var cachedModule = Module._cache[filename];
440442
if (cachedModule) {
443+
updateChildren(parent, cachedModule, true);
441444
return cachedModule.exports;
442445
}
443446

@@ -446,6 +449,7 @@ Module._load = function(request, parent, isMain) {
446449
return NativeModule.require(filename);
447450
}
448451

452+
// Don't call updateChildren(), Module constructor already does.
449453
var module = new Module(filename, parent);
450454

451455
if (isMain) {

test/fixtures/GH-7131/a.js

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
'use strict';
2+
require('sys'); // Builtin should not show up in module.children array.
3+
require('./b'); // This should.
4+
require('./b'); // This should not.
5+
module.exports = module.children.slice();

test/fixtures/GH-7131/b.js

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
'use strict';
2+
module.exports = module.children.slice();

test/parallel/test-module-children.js

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Flags: --no-deprecation
2+
'use strict';
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const path = require('path');
6+
7+
const dir = path.join(common.fixturesDir, 'GH-7131');
8+
const b = require(path.join(dir, 'b'));
9+
const a = require(path.join(dir, 'a'));
10+
11+
assert.strictEqual(a.length, 1);
12+
assert.strictEqual(b.length, 0);
13+
assert.deepStrictEqual(a[0].exports, b);

test/sequential/test-module-loading.js

+3
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,10 @@ try {
237237
// modules that we've required, and that all of them contain
238238
// the appropriate children, and so on.
239239

240+
const visited = new Set();
240241
const children = module.children.reduce(function red(set, child) {
242+
if (visited.has(child)) return set;
243+
visited.add(child);
241244
let id = path.relative(path.dirname(__dirname), child.id);
242245
id = id.replace(backslash, '/');
243246
set[id] = child.children.reduce(red, {});

0 commit comments

Comments
 (0)