From 8d7330be0e8a6952438c4671a84d361568b621b2 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

Backport-PR-URL: https://github.com/nodejs/node/pull/33533
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                            | 12 +++++--
 lib/internal/modules/cjs/loader.js            |  3 +-
 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 +-
 8 files changed, 51 insertions(+), 9 deletions(-)

diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md
index 842ebf97ec5aff..58168640cec6cc 100644
--- a/doc/api/deprecations.md
+++ b/doc/api/deprecations.md
@@ -2707,6 +2707,39 @@ Type: Runtime
 `Transform._transformState` will be removed in future versions where it is
 no longer required due to simplification of the implementation.
 
+<a id="DEP0144"></a>
+### DEP0144: `module.parent`
+<!-- YAML
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/32217
+    description: Documentation-only deprecation.
+-->
+
+Type: Documentation-only
+
+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));
+```
+
 <a id="DEP0XXX"></a>
 ### DEP0XXX: `socket.bufferSize`
 <!-- YAML
diff --git a/doc/api/modules.md b/doc/api/modules.md
index 1d4c429529ec05..eb6903cbc6a281 100644
--- a/doc/api/modules.md
+++ b/doc/api/modules.md
@@ -905,11 +905,17 @@ loading.
 ### `module.parent`
 <!-- YAML
 added: v0.1.16
+deprecated: REPLACEME
 -->
 
-* {module}
+> Stability: 0 - Deprecated: Please use [`require.main`][] and
+> [`module.children`][] instead.
+
+* {module | null | undefined}
 
-The module that first required this one.
+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`).
 
 ### `module.path`
 <!-- YAML
@@ -1133,6 +1139,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
@@ -1140,6 +1147,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 96cfffe90a0d50..1f672dd1ff2d4d 100644
--- a/lib/internal/modules/cjs/loader.js
+++ b/lib/internal/modules/cjs/loader.js
@@ -1247,7 +1247,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 } = 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 1d4d7450ed4b18..6d191409ad4d7f 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 f55c1a67c49ec2..7aba4fa6d825a2 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 986f644fd226b9..23efb32678eb87 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 7dc3f4b3176877..7f8e785ac43546 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 cbe0a09887f8eb..d408f09bb9dff6 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.');