module: warn on using unfinished circular dependency

Warn when a non-existent property of an unfinished module.exports
object is being accessed, as that very often indicates the presence
of a hard-to-detect and hard-to-debug problem.

This mechanism is only used if `module.exports` is still a
regular object at the point at which the second, circular `require()`
happens.

The downside is that, temporarily, `module.exports` will have a
prototype other than `Object.prototype`, and that there may
be valid uses of accessing non-existent properties of unfinished
`module.exports` objects.

Performance of circular require calls in general is not
noticeably impacted.

                                               confidence improvement accuracy (*)   (**)  (***)
     module/module-loader-circular.js n=10000                 3.96 %       ±5.12% ±6.82% ±8.89%

Example:

    $ cat a.js
    'use strict';
    const b = require('./b.js');

    exports.fn = () => {};
    $ cat b.js
    'use strict';
    const a = require('./a.js');

    a.fn();
    $ node a.js
    (node:1617) Warning: Accessing non-existent property 'fn' of module exports inside circular dependency
    /tmp/b.js:4
    a.fn();
      ^

    TypeError: a.fn is not a function
        at Object.<anonymous> (/tmp/b.js:4:3)
        [...]

PR-URL: https://github.com/nodejs/node/pull/29935
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

8 files changed, 22 insertions, 0 deletions

diff --git a/test/fixtures/cycles/warning-a.js b/test/fixtures/cycles/warning-a.js
new file mode 100644
index 0000000000..dea4c53a26
--- /dev/null
+++ b/test/fixtures/cycles/warning-a.js
@@ -0,0 +1 @@
+require('./warning-b.js');
diff --git a/test/fixtures/cycles/warning-b.js b/test/fixtures/cycles/warning-b.js
new file mode 100644
index 0000000000..3be73bfd0a
--- /dev/null
+++ b/test/fixtures/cycles/warning-b.js
@@ -0,0 +1,3 @@
+const a = require('./warning-a.js');
+a.missingPropB;
+a[Symbol('someSymbol')];
diff --git a/test/fixtures/cycles/warning-esm-transpiled-a.js b/test/fixtures/cycles/warning-esm-transpiled-a.js
new file mode 100644
index 0000000000..fe70e745d3
--- /dev/null
+++ b/test/fixtures/cycles/warning-esm-transpiled-a.js
@@ -0,0 +1,2 @@
+Object.defineProperty(exports, "__esModule", { value: true });
+require('./warning-esm-transpiled-b.js');
diff --git a/test/fixtures/cycles/warning-esm-transpiled-b.js b/test/fixtures/cycles/warning-esm-transpiled-b.js
new file mode 100644
index 0000000000..a44a63ce2c
--- /dev/null
+++ b/test/fixtures/cycles/warning-esm-transpiled-b.js
@@ -0,0 +1 @@
+require('./warning-esm-transpiled-a.js').missingPropESM;
diff --git a/test/fixtures/cycles/warning-moduleexports-a.js b/test/fixtures/cycles/warning-moduleexports-a.js
new file mode 100644
index 0000000000..b37504b1b8
--- /dev/null
+++ b/test/fixtures/cycles/warning-moduleexports-a.js
@@ -0,0 +1,2 @@
+module.exports = {};
+require('./warning-moduleexports-b.js');
diff --git a/test/fixtures/cycles/warning-moduleexports-b.js b/test/fixtures/cycles/warning-moduleexports-b.js
new file mode 100644
index 0000000000..8d2292934d
--- /dev/null
+++ b/test/fixtures/cycles/warning-moduleexports-b.js
@@ -0,0 +1 @@
+require('./warning-moduleexports-b.js').missingPropModuleExportsB;
diff --git a/test/fixtures/cycles/warning-moduleexports-class-a.js b/test/fixtures/cycles/warning-moduleexports-class-a.js
new file mode 100644
index 0000000000..e23654d7f3
--- /dev/null
+++ b/test/fixtures/cycles/warning-moduleexports-class-a.js
@@ -0,0 +1,11 @@
+const assert = require('assert');
+
+class Parent {}
+class A extends Parent {}
+
+module.exports = A;
+require('./warning-moduleexports-class-b.js');
+process.nextTick(() => {
+  assert.strictEqual(module.exports, A);
+  assert.strictEqual(Object.getPrototypeOf(module.exports), Parent);
+});
diff --git a/test/fixtures/cycles/warning-moduleexports-class-b.js b/test/fixtures/cycles/warning-moduleexports-class-b.js
new file mode 100644
index 0000000000..3fe1763eb7
--- /dev/null
+++ b/test/fixtures/cycles/warning-moduleexports-class-b.js
@@ -0,0 +1 @@
+require('./warning-moduleexports-class-a.js').missingPropModuleExportsClassB;