FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

module: do not check circular dependencies for exported proxies · nodejs/node@c2d2dfc · GitHub

/ node Public

Commit c2d2dfc

Browse files
BridgeAR
authored and
codebytere
committed
module: do not check circular dependencies for exported proxies
In case the exported module is a proxy that has the `getPrototypeOf` or `setPrototypeOf` trap, skip the circular dependencies check. It would otherwise be triggered by the check itself. Fixes: #33334 Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de> PR-URL: #33338 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Zeyu Yang <himself65@outlook.com>
1 parent 6257cf2 commit c2d2dfc

4 files changed

Lines changed: 45 additions & 5 deletions

File tree

‎lib/internal/modules/cjs/loader.js‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,11 @@ const {
107107
CHAR_COLON
108108
} = require('internal/constants');
109109

110+
111+
const {
112+
isProxy
113+
} = require('internal/util/types');
114+
110115
const isWindows = process.platform === 'win32';
111116

112117
const relativeResolveCache = ObjectCreate(null);
@@ -821,7 +826,7 @@ function emitCircularRequireWarning(prop) {
821826
}
822827

823828
// A Proxy that can be used as the prototype of a module.exports object and
824-
// warns when non-existend properties are accessed.
829+
// warns when non-existent properties are accessed.
825830
const CircularRequirePrototypeWarningProxy = new Proxy({}, {
826831
get(target, prop) {
827832
// Allow __esModule access in any case because it is used in the output
@@ -840,20 +845,22 @@ const CircularRequirePrototypeWarningProxy = new Proxy({}, {
840845
}
841846
});
842847

843-
// Object.prototype and ObjectProtoype refer to our 'primordials' versions
848+
// Object.prototype and ObjectPrototype refer to our 'primordials' versions
844849
// and are not identical to the versions on the global object.
845850
const PublicObjectPrototype = global.Object.prototype;
846851

847852
function getExportsForCircularRequire(module) {
848853
if (module.exports &&
854+
!isProxy(module.exports) &&
849855
ObjectGetPrototypeOf(module.exports) === PublicObjectPrototype &&
850856
// Exclude transpiled ES6 modules / TypeScript code because those may
851-
// employ unusual patterns for accessing 'module.exports'. That should be
852-
// okay because ES6 modules have a different approach to circular
857+
// employ unusual patterns for accessing 'module.exports'. That should
858+
// be okay because ES6 modules have a different approach to circular
853859
// dependencies anyway.
854860
!module.exports.__esModule) {
855861
// This is later unset once the module is done loading.
856-
ObjectSetPrototypeOf(module.exports, CircularRequirePrototypeWarningProxy);
862+
ObjectSetPrototypeOf(
863+
module.exports, CircularRequirePrototypeWarningProxy);
857864
}
858865

859866
return module.exports;
@@ -943,6 +950,7 @@ Module._load = function(request, parent, isMain) {
943950
}
944951
}
945952
} else if (module.exports &&
953+
!isProxy(module.exports) &&
946954
ObjectGetPrototypeOf(module.exports) ===
947955
CircularRequirePrototypeWarningProxy) {
948956
ObjectSetPrototypeOf(module.exports, PublicObjectPrototype);

‎test/fixtures/cycles/warning-skip-proxy-traps-a.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
module.exports = new Proxy({}, {
2+
get(_target, prop) { throw new Error(`get: ${String(prop)}`); },
3+
getPrototypeOf() { throw new Error('getPrototypeOf'); },
4+
setPrototypeOf() { throw new Error('setPrototypeOf'); },
5+
isExtensible() { throw new Error('isExtensible'); },
6+
preventExtensions() { throw new Error('preventExtensions'); },
7+
getOwnPropertyDescriptor() { throw new Error('getOwnPropertyDescriptor'); },
8+
defineProperty() { throw new Error('defineProperty'); },
9+
has() { throw new Error('has'); },
10+
set() { throw new Error('set'); },
11+
deleteProperty() { throw new Error('deleteProperty'); },
12+
ownKeys() { throw new Error('ownKeys'); },
13+
apply() { throw new Error('apply'); },
14+
construct() { throw new Error('construct'); }
15+
});
16+
17+
require('./warning-skip-proxy-traps-b.js');

‎test/fixtures/cycles/warning-skip-proxy-traps-b.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
const assert = require('assert');
2+
3+
const object = require('./warning-skip-proxy-traps-a.js');
4+
5+
assert.throws(() => {
6+
object.missingPropProxyTrap;
7+
}, {
8+
message: 'get: missingPropProxyTrap',
9+
name: 'Error',
10+
});

‎test/parallel/test-module-circular-dependency-warning.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,8 @@ assert.strictEqual(esmTranspiledExport.__esModule, true);
3838
const halfTranspiledExport =
3939
require(fixtures.path('cycles', 'warning-esm-half-transpiled-a.js'));
4040
assert.strictEqual(halfTranspiledExport.__esModule, undefined);
41+
42+
// No circular check is done to prevent triggering proxy traps, if
43+
// module.exports is set to a proxy that contains a `getPrototypeOf` or
44+
// `setPrototypeOf` trap.
45+
require(fixtures.path('cycles', 'warning-skip-proxy-traps-a.js'));

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL