From c83d9bbffbe879f9d67f72c14213139616ec4302 Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Sat, 8 Jul 2017 13:06:02 +0200
Subject: [PATCH] lib: include cached modules in module.children
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`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: https://github.com/nodejs/node/issues/7131
PR-URL: https://github.com/nodejs/node/pull/14132
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
---
 lib/module.js                          | 12 ++++++++----
 test/fixtures/GH-7131/a.js             |  5 +++++
 test/fixtures/GH-7131/b.js             |  2 ++
 test/parallel/test-module-children.js  | 13 +++++++++++++
 test/sequential/test-module-loading.js |  3 +++
 5 files changed, 31 insertions(+), 4 deletions(-)
 create mode 100644 test/fixtures/GH-7131/a.js
 create mode 100644 test/fixtures/GH-7131/b.js
 create mode 100644 test/parallel/test-module-children.js

diff --git a/lib/module.js b/lib/module.js
index ae6f4ff453..339a228da9 100644
--- a/lib/module.js
+++ b/lib/module.js
@@ -46,15 +46,17 @@ function stat(filename) {
 }
 stat.cache = null;
 
+function updateChildren(parent, child, scan) {
+  var children = parent && parent.children;
+  if (children && !(scan && children.includes(child)))
+    children.push(child);
+}
 
 function Module(id, parent) {
   this.id = id;
   this.exports = {};
   this.parent = parent;
-  if (parent && parent.children) {
-    parent.children.push(this);
-  }
-
+  updateChildren(parent, this, false);
   this.filename = null;
   this.loaded = false;
   this.children = [];
@@ -438,6 +440,7 @@ Module._load = function(request, parent, isMain) {
 
   var cachedModule = Module._cache[filename];
   if (cachedModule) {
+    updateChildren(parent, cachedModule, true);
     return cachedModule.exports;
   }
 
@@ -446,6 +449,7 @@ Module._load = function(request, parent, isMain) {
     return NativeModule.require(filename);
   }
 
+  // Don't call updateChildren(), Module constructor already does.
   var module = new Module(filename, parent);
 
   if (isMain) {
diff --git a/test/fixtures/GH-7131/a.js b/test/fixtures/GH-7131/a.js
new file mode 100644
index 0000000000..8c386a4d61
--- /dev/null
+++ b/test/fixtures/GH-7131/a.js
@@ -0,0 +1,5 @@
+'use strict';
+require('sys');  // Builtin should not show up in module.children array.
+require('./b');  // This should.
+require('./b');  // This should not.
+module.exports = module.children.slice();
diff --git a/test/fixtures/GH-7131/b.js b/test/fixtures/GH-7131/b.js
new file mode 100644
index 0000000000..0e7caf3663
--- /dev/null
+++ b/test/fixtures/GH-7131/b.js
@@ -0,0 +1,2 @@
+'use strict';
+module.exports = module.children.slice();
diff --git a/test/parallel/test-module-children.js b/test/parallel/test-module-children.js
new file mode 100644
index 0000000000..9eec14fda6
--- /dev/null
+++ b/test/parallel/test-module-children.js
@@ -0,0 +1,13 @@
+// Flags: --no-deprecation
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const path = require('path');
+
+const dir = path.join(common.fixturesDir, 'GH-7131');
+const b = require(path.join(dir, 'b'));
+const a = require(path.join(dir, 'a'));
+
+assert.strictEqual(a.length, 1);
+assert.strictEqual(b.length, 0);
+assert.deepStrictEqual(a[0].exports, b);
diff --git a/test/sequential/test-module-loading.js b/test/sequential/test-module-loading.js
index b117555a22..e59ec24782 100644
--- a/test/sequential/test-module-loading.js
+++ b/test/sequential/test-module-loading.js
@@ -237,7 +237,10 @@ try {
   // modules that we've required, and that all of them contain
   // the appropriate children, and so on.
 
+  const visited = new Set();
   const children = module.children.reduce(function red(set, child) {
+    if (visited.has(child)) return set;
+    visited.add(child);
     let id = path.relative(path.dirname(__dirname), child.id);
     id = id.replace(backslash, '/');
     set[id] = child.children.reduce(red, {});