module: warn on using unfinished circular dependency by addaleax · Pull Request #29935

module: warn on using unfinished circular dependency by addaleax · Pull Request #29935 · nodejs/node

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)
        [...]

added the module

Issues and PRs related to the module subsystem.

label

Oct 11, 2019

addaleax added a commit to addaleax/cli that referenced this pull request

Oct 11, 2019

`lib/pack.js` and `lib/config/figgy-config.js` load each other,
making `figgy-config.js` grab the original `module.exports` value
and not the intended one. In particular, this always sets the
`dirPacker` value to `undefined` in the config generation step.

Fix this by setting `module.exports` early.

Refs: nodejs/node#29935

addaleax added a commit to addaleax/shelljs that referenced this pull request

Oct 18, 2019

Node.js is currently considering printing a warning when a non-existent
property of `module.exports` is accessed while in a circular `require()`
dependency, in order to make it easier to catch issues with circular
dependencies.

In order to avoid printing these warnings for shelljs, checking for the
property’s existence rather than its truthiness suffices.

Refs: nodejs/node#29935

nfischer pushed a commit to shelljs/shelljs that referenced this pull request

Oct 20, 2019

Node.js is currently considering printing a warning when a non-existent
property of `module.exports` is accessed while in a circular `require()`
dependency, in order to make it easier to catch issues with circular
dependencies.

In order to avoid printing these warnings for shelljs, checking for the
property’s existence rather than its truthiness suffices.

Refs: nodejs/node#29935

addaleax added the semver-major

PRs that contain breaking changes and should be released in the next major version.

label

Oct 23, 2019

addaleax added a commit to addaleax/node that referenced this pull request

Apr 24, 2020

Since this property access is performed by generated code, and not
used for accessing the actual exports of a module (and because
transpilers generally define it as the first key of `module.exports`
when it *is* present), it should be okay to allow it.

Refs: nodejs#29935
Fixes: nodejs#33046

nfischer pushed a commit to shelljs/shelljs that referenced this pull request

Apr 25, 2020

Node.js is currently considering printing a warning when a non-existent
property of `module.exports` is accessed while in a circular `require()`
dependency, in order to make it easier to catch issues with circular
dependencies.

In order to avoid printing these warnings for shelljs, checking for the
property’s existence rather than its truthiness suffices.

Refs: nodejs/node#29935

addaleax added a commit that referenced this pull request

Apr 27, 2020

Since this property access is performed by generated code, and not
used for accessing the actual exports of a module (and because
transpilers generally define it as the first key of `module.exports`
when it *is* present), it should be okay to allow it.

Refs: #29935
Fixes: #33046

PR-URL: #33048
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>

BethGriggs pushed a commit that referenced this pull request

Apr 27, 2020

Since this property access is performed by generated code, and not
used for accessing the actual exports of a module (and because
transpilers generally define it as the first key of `module.exports`
when it *is* present), it should be okay to allow it.

Refs: #29935
Fixes: #33046

PR-URL: #33048
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>

BethGriggs pushed a commit that referenced this pull request

Apr 28, 2020

Since this property access is performed by generated code, and not
used for accessing the actual exports of a module (and because
transpilers generally define it as the first key of `module.exports`
when it *is* present), it should be okay to allow it.

Refs: #29935
Fixes: #33046

PR-URL: #33048
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>

BethGriggs pushed a commit that referenced this pull request

Apr 28, 2020

Since this property access is performed by generated code, and not
used for accessing the actual exports of a module (and because
transpilers generally define it as the first key of `module.exports`
when it *is* present), it should be okay to allow it.

Refs: #29935
Fixes: #33046

PR-URL: #33048
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>

This was referenced

Jun 7, 2020

DrumBeatRhythm added a commit to DrumBeatRhythm/fwf that referenced this pull request

Jul 26, 2025

Node.js is currently considering printing a warning when a non-existent
property of `module.exports` is accessed while in a circular `require()`
dependency, in order to make it easier to catch issues with circular
dependencies.

In order to avoid printing these warnings for shelljs, checking for the
property’s existence rather than its truthiness suffices.

Refs: nodejs/node#29935

MathematicsMelode added a commit to MathematicsMelode/Injector that referenced this pull request

Jul 31, 2025

Node.js is currently considering printing a warning when a non-existent
property of `module.exports` is accessed while in a circular `require()`
dependency, in order to make it easier to catch issues with circular
dependencies.

In order to avoid printing these warnings for shelljs, checking for the
property’s existence rather than its truthiness suffices.

Refs: nodejs/node#29935

MosseMusing added a commit to MosseMusing/TOTALLY that referenced this pull request

Aug 13, 2025

Node.js is currently considering printing a warning when a non-existent
property of `module.exports` is accessed while in a circular `require()`
dependency, in order to make it easier to catch issues with circular
dependencies.

In order to avoid printing these warnings for shelljs, checking for the
property’s existence rather than its truthiness suffices.

Refs: nodejs/node#29935

987AI added a commit to 987AI/friendle that referenced this pull request

Sep 17, 2025

Node.js is currently considering printing a warning when a non-existent
property of `module.exports` is accessed while in a circular `require()`
dependency, in order to make it easier to catch issues with circular
dependencies.

In order to avoid printing these warnings for shelljs, checking for the
property’s existence rather than its truthiness suffices.

Refs: nodejs/node#29935

1sjohnson added a commit to 1sjohnson/nicein that referenced this pull request

Sep 20, 2025

Node.js is currently considering printing a warning when a non-existent
property of `module.exports` is accessed while in a circular `require()`
dependency, in order to make it easier to catch issues with circular
dependencies.

In order to avoid printing these warnings for shelljs, checking for the
property’s existence rather than its truthiness suffices.

Refs: nodejs/node#29935

paulluome added a commit to paulluome/FeeDate that referenced this pull request

Sep 23, 2025

Node.js is currently considering printing a warning when a non-existent
property of `module.exports` is accessed while in a circular `require()`
dependency, in order to make it easier to catch issues with circular
dependencies.

In order to avoid printing these warnings for shelljs, checking for the
property’s existence rather than its truthiness suffices.

Refs: nodejs/node#29935

catLewisix added a commit to catLewisix/Iranian that referenced this pull request

Sep 26, 2025

Node.js is currently considering printing a warning when a non-existent
property of `module.exports` is accessed while in a circular `require()`
dependency, in order to make it easier to catch issues with circular
dependencies.

In order to avoid printing these warnings for shelljs, checking for the
property’s existence rather than its truthiness suffices.

Refs: nodejs/node#29935

dabbertorre added a commit to dabbertorre/mutil that referenced this pull request

Sep 26, 2025

Node.js is currently considering printing a warning when a non-existent
property of `module.exports` is accessed while in a circular `require()`
dependency, in order to make it easier to catch issues with circular
dependencies.

In order to avoid printing these warnings for shelljs, checking for the
property’s existence rather than its truthiness suffices.

Refs: nodejs/node#29935

Alessandio added a commit to Alessandio/workshop that referenced this pull request

Dec 14, 2025

Node.js is currently considering printing a warning when a non-existent
property of `module.exports` is accessed while in a circular `require()`
dependency, in order to make it easier to catch issues with circular
dependencies.

In order to avoid printing these warnings for shelljs, checking for the
property’s existence rather than its truthiness suffices.

Refs: nodejs/node#29935

imSerenae added a commit to imSerenae/mqtt that referenced this pull request

Jan 9, 2026

Node.js is currently considering printing a warning when a non-existent
property of `module.exports` is accessed while in a circular `require()`
dependency, in order to make it easier to catch issues with circular
dependencies.

In order to avoid printing these warnings for shelljs, checking for the
property’s existence rather than its truthiness suffices.

Refs: nodejs/node#29935

RedBearLabe added a commit to RedBearLabe/BLENano that referenced this pull request

Jan 26, 2026

Node.js is currently considering printing a warning when a non-existent
property of `module.exports` is accessed while in a circular `require()`
dependency, in order to make it easier to catch issues with circular
dependencies.

In order to avoid printing these warnings for shelljs, checking for the
property’s existence rather than its truthiness suffices.

Refs: nodejs/node#29935

downwardegardner added a commit to downwardegardner/moddy that referenced this pull request

Feb 8, 2026

Node.js is currently considering printing a warning when a non-existent
property of `module.exports` is accessed while in a circular `require()`
dependency, in order to make it easier to catch issues with circular
dependencies.

In order to avoid printing these warnings for shelljs, checking for the
property’s existence rather than its truthiness suffices.

Refs: nodejs/node#29935