From 6443ab9595de9e7f57144f063e42efb73e34d714 Mon Sep 17 00:00:00 2001
From: Antoine du HAMEL <duhamelantoine1995@gmail.com>
Date: Wed, 11 Mar 2020 23:44:31 +0100
Subject: [PATCH] module: deprecate module.parent
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This feature does not work when a module is imported using ECMAScript
modules specification, therefore it is deprecated.

Fixes: https://github.com/nodejs/modules/issues/469

PR-URL: https://github.com/nodejs/node/pull/32217
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
---
 doc/api/deprecations.md                       | 33 +++++++++++++++++++
 doc/api/modules.md                            | 13 ++++++--
 lib/internal/modules/cjs/loader.js            | 21 ++++++++++--
 test/common/index.js                          |  4 +--
 test/common/require-as.js                     |  2 +-
 test/js-native-api/test_instance_data/test.js |  2 +-
 test/node-api/test_instance_data/test.js      |  2 +-
 test/parallel/test-cli-eval.js                |  2 +-
 .../test-module-parent-deprecation.js         | 14 ++++++++
 9 files changed, 81 insertions(+), 12 deletions(-)
 create mode 100644 test/parallel/test-module-parent-deprecation.js

diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md
index 59232512d4c..630ae8fd5b9 100644
--- a/doc/api/deprecations.md
+++ b/doc/api/deprecations.md
@@ -2712,6 +2712,39 @@ The `repl` module exports a `_builtinLibs` property that contains an array with
 native modules. It was incomplete so far and instead it's better to rely upon
 `require('module').builtinModules`.
 
+<a id="DEP0143"></a>
+### DEP0143: `module.parent`
+<!-- YAML
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/32217
+    description: Documentation-only deprecation.
+-->
+
+Type: Documentation-only (supports [`--pending-deprecation`][])
+
+A CommonJS module can access the first module that required it using
+`module.parent`. This feature is deprecated because it does not work
+consistently in the presence of ECMAScript modules and because it gives an
+inaccurate representation of the CommonJS module graph.
+
+Some modules use it to check if they are the entry point of the current process.
+Instead, it is recommended to compare `require.main` and `module`:
+
+```js
+if (require.main === module) {
+  // Code section that will run only if current file is the entry point.
+}
+```
+
+When looking for the CommonJS modules that have required the current one,
+`require.cache` and `module.children` can be used:
+
+```js
+const moduleParents = Object.values(require.cache)
+  .filter((m) => m.children.includes(module));
+```
+
 [`--pending-deprecation`]: cli.html#cli_pending_deprecation
 [`--throw-deprecation`]: cli.html#cli_throw_deprecation
 [`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size
diff --git a/doc/api/modules.md b/doc/api/modules.md
index c57aa69c6d0..0718809d99b 100644
--- a/doc/api/modules.md
+++ b/doc/api/modules.md
@@ -690,7 +690,6 @@ Module {
   id: '.',
   path: '/absolute/path/to',
   exports: {},
-  parent: null,
   filename: '/absolute/path/to/entry.js',
   loaded: false,
   children: [],
@@ -894,11 +893,17 @@ loading.
 ### `module.parent`
 <!-- YAML
 added: v0.1.16
+deprecated: REPLACEME
 -->
 
-* {module}
+> Stability: 0 - Deprecated: Please use [`require.main`][] and
+> [`module.children`][] instead.
 
-The module that first required this one.
+* {module | null | undefined}
+
+The module that first required this one, or `null` if the current module is the
+entry point of the current process, or `undefined` if the module was loaded by
+something that is not a CommonJS module (E.G.: REPL or `import`). Read only.
 
 ### `module.path`
 <!-- YAML
@@ -1122,6 +1127,7 @@ consists of the following keys:
 [`createRequire()`]: #modules_module_createrequire_filename
 [`module` object]: #modules_the_module_object
 [`module.id`]: #modules_module_id
+[`module.children`]: #modules_module_children
 [`path.dirname()`]: path.html#path_path_dirname_path
 [ECMAScript Modules]: esm.html
 [an error]: errors.html#errors_err_require_esm
@@ -1129,6 +1135,7 @@ consists of the following keys:
 [module resolution]: #modules_all_together
 [module wrapper]: #modules_the_module_wrapper
 [native addons]: addons.html
+[`require.main`]: #modules_require_main
 [source map include directives]: https://sourcemaps.info/spec.html#h.lmz475t4mvbx
 [`--enable-source-maps`]: cli.html#cli_enable_source_maps
 [`NODE_V8_COVERAGE=dir`]: cli.html#cli_node_v8_coverage_dir
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
index 35eabb55f2b..ffe8173aeb4 100644
--- a/lib/internal/modules/cjs/loader.js
+++ b/lib/internal/modules/cjs/loader.js
@@ -39,6 +39,7 @@ const {
   ReflectSet,
   RegExpPrototypeTest,
   SafeMap,
+  SafeWeakMap,
   String,
   StringPrototypeIndexOf,
   StringPrototypeLastIndexOf,
@@ -160,11 +161,12 @@ function updateChildren(parent, child, scan) {
     children.push(child);
 }
 
+const moduleParentCache = new SafeWeakMap();
 function Module(id = '', parent) {
   this.id = id;
   this.path = path.dirname(id);
   this.exports = {};
-  this.parent = parent;
+  moduleParentCache.set(this, parent);
   updateChildren(parent, this, false);
   this.filename = null;
   this.loaded = false;
@@ -233,6 +235,18 @@ ObjectDefineProperty(Module, 'wrapper', {
   }
 });
 
+function getModuleParent() {
+  return moduleParentCache.get(this);
+}
+ObjectDefineProperty(Module.prototype, 'parent', {
+  get: pendingDeprecation ? deprecate(
+    getModuleParent,
+    'module.parent is deprecated due to accuracy issues. Please use ' +
+      'require.main to find program entry point instead.',
+    'DEP0143'
+  ) : getModuleParent
+});
+
 const debug = require('internal/util/debuglog').debuglog('module');
 Module._debug = deprecate(debug, 'Module._debug is deprecated.', 'DEP0077');
 
@@ -1020,7 +1034,7 @@ Module._resolveFilename = function(request, parent, isMain, options) {
   const requireStack = [];
   for (let cursor = parent;
     cursor;
-    cursor = cursor.parent) {
+    cursor = moduleParentCache.get(cursor)) {
     requireStack.push(cursor.filename || cursor.id);
   }
   let message = `Cannot find module '${request}'`;
@@ -1213,7 +1227,8 @@ Module._extensions['.js'] = function(module, filename) {
     const pkg = readPackageScope(filename);
     // Function require shouldn't be used in ES modules.
     if (pkg && pkg.data && pkg.data.type === 'module') {
-      const parentPath = module.parent && module.parent.filename;
+      const parent = moduleParentCache.get(module);
+      const parentPath = parent && parent.filename;
       const packageJsonPath = path.resolve(pkg.path, 'package.json');
       throw new ERR_REQUIRE_ESM(filename, parentPath, packageJsonPath);
     }
diff --git a/test/common/index.js b/test/common/index.js
index 6343425858d..f7a29f5d2f4 100644
--- a/test/common/index.js
+++ b/test/common/index.js
@@ -59,12 +59,12 @@ if (process.argv.length === 2 &&
     !process.env.NODE_SKIP_FLAG_CHECK &&
     isMainThread &&
     hasCrypto &&
-    module.parent &&
+    require.main &&
     require('cluster').isMaster) {
   // The copyright notice is relatively big and the flags could come afterwards.
   const bytesToRead = 1500;
   const buffer = Buffer.allocUnsafe(bytesToRead);
-  const fd = fs.openSync(module.parent.filename, 'r');
+  const fd = fs.openSync(require.main.filename, 'r');
   const bytesRead = fs.readSync(fd, buffer, 0, bytesToRead);
   fs.closeSync(fd);
   const source = buffer.toString('utf8', 0, bytesRead);
diff --git a/test/common/require-as.js b/test/common/require-as.js
index f55c1a67c49..7aba4fa6d82 100644
--- a/test/common/require-as.js
+++ b/test/common/require-as.js
@@ -1,7 +1,7 @@
 /* eslint-disable node-core/require-common-first, node-core/required-modules */
 'use strict';
 
-if (module.parent) {
+if (require.main !== module) {
   const { spawnSync } = require('child_process');
 
   function runModuleAs(filename, flags, spawnOptions, role) {
diff --git a/test/js-native-api/test_instance_data/test.js b/test/js-native-api/test_instance_data/test.js
index 986f644fd22..23efb32678e 100644
--- a/test/js-native-api/test_instance_data/test.js
+++ b/test/js-native-api/test_instance_data/test.js
@@ -4,7 +4,7 @@
 const common = require('../../common');
 const assert = require('assert');
 
-if (module.parent) {
+if (module !== require.main) {
   // When required as a module, run the tests.
   const test_instance_data =
     require(`./build/${common.buildType}/test_instance_data`);
diff --git a/test/node-api/test_instance_data/test.js b/test/node-api/test_instance_data/test.js
index 7dc3f4b3176..7f8e785ac43 100644
--- a/test/node-api/test_instance_data/test.js
+++ b/test/node-api/test_instance_data/test.js
@@ -3,7 +3,7 @@
 
 const common = require('../../common');
 
-if (module.parent) {
+if (module !== require.main) {
   // When required as a module, run the tests.
   const test_instance_data =
     require(`./build/${common.buildType}/test_instance_data`);
diff --git a/test/parallel/test-cli-eval.js b/test/parallel/test-cli-eval.js
index cbe0a09887f..d408f09bb9d 100644
--- a/test/parallel/test-cli-eval.js
+++ b/test/parallel/test-cli-eval.js
@@ -20,7 +20,7 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 'use strict';
-if (module.parent) {
+if (module !== require.main) {
   // Signal we've been loaded as a module.
   // The following console.log() is part of the test.
   console.log('Loaded as a module, exiting with status code 42.');
diff --git a/test/parallel/test-module-parent-deprecation.js b/test/parallel/test-module-parent-deprecation.js
new file mode 100644
index 00000000000..439e86bc968
--- /dev/null
+++ b/test/parallel/test-module-parent-deprecation.js
@@ -0,0 +1,14 @@
+// Flags: --pending-deprecation
+
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+
+common.expectWarning(
+  'DeprecationWarning',
+  'module.parent is deprecated due to accuracy issues. Please use ' +
+    'require.main to find program entry point instead.',
+  'DEP0143'
+);
+
+assert.strictEqual(module.parent, null);