Skip to content

Commit

Permalink
lib: do not throw if global property is no longer configurable
Browse files Browse the repository at this point in the history
Fixes: #45336
PR-URL: #45344
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
aduh95 authored and danielleadams committed Jan 3, 2023
1 parent 8b44c61 commit 2c595da
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 10 deletions.
23 changes: 13 additions & 10 deletions lib/internal/modules/cjs/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,16 +176,19 @@ function addBuiltinLibsToObject(object, dummyModuleName) {
get: () => {
const lib = dummyModule.require(name);

// Disable the current getter/setter and set up a new
// non-enumerable property.
delete object[name];
ObjectDefineProperty(object, name, {
__proto__: null,
get: () => lib,
set: setReal,
configurable: true,
enumerable: false
});
try {
// Override the current getter/setter and set up a new
// non-enumerable property.
ObjectDefineProperty(object, name, {
__proto__: null,
get: () => lib,
set: setReal,
configurable: true,
enumerable: false,
});
} catch {
// If the property is no longer configurable, ignore the error.
}

return lib;
},
Expand Down
9 changes: 9 additions & 0 deletions test/parallel/test-cli-eval.js
Original file line number Diff line number Diff line change
Expand Up @@ -288,3 +288,12 @@ child.exec(
common.mustSucceed((stdout) => {
assert.strictEqual(stdout, '.mjs file\n');
}));

// Regression test for https://github.com/nodejs/node/issues/45336
child.execFile(process.execPath,
['-p',
'Object.defineProperty(global, "fs", { configurable: false });' +
'fs === require("node:fs")'],
common.mustSucceed((stdout) => {
assert.match(stdout, /^true/);
}));

0 comments on commit 2c595da

Please sign in to comment.