lib: do not throw if global property is no longer configurable · nodejs/node@38767b4 · GitHub

nodejs / node Public

• Notifications You must be signed in to change notification settings
• Fork 36.6k
• Star 120k

• Code
• Issues 633
• Pull requests 592
• Actions
• Projects
• Security and quality
• Insights

Additional navigation options

Commit 38767b4

Browse filesBrowse the repository at this point in the historyBrowse files

aduh95

authored and

ruyadorno

committed

lib: do not throw if global property is no longer configurable

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>

1 parent 4de67d1 commit 38767b4
Copy full SHA for 38767b4

2 files changed

+22-10Lines changed: 22 additions & 10 deletions

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

• lib/internal/modules/cjs
  • helpers.js
• test/parallel
  • test-cli-eval.js

Expand file treeCollapse file tree

Open diff view settings

Collapse file

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

Copy file name to clipboardExpand all lines: lib/internal/modules/cjs/helpers.js

+13-10Lines changed: 13 additions & 10 deletions

Original file line number	Diff line number	Diff line change
@@ -176,16 +176,19 @@ function addBuiltinLibsToObject(object, dummyModuleName) {
176	176	get: () => {
177	177	const lib = dummyModule.require(name);
178	178
179		- // Disable the current getter/setter and set up a new
180		- // non-enumerable property.
181		- delete object[name];
182		- ObjectDefineProperty(object, name, {
183		- __proto__: null,
184		- get: () => lib,
185		- set: setReal,
186		- configurable: true,
187		- enumerable: false
188		- });
	179	+ try {
	180	+ // Override the current getter/setter and set up a new
	181	+ // non-enumerable property.
	182	+ ObjectDefineProperty(object, name, {
	183	+ __proto__: null,
	184	+ get: () => lib,
	185	+ set: setReal,
	186	+ configurable: true,
	187	+ enumerable: false,
	188	+ });
	189	+ } catch {
	190	+ // If the property is no longer configurable, ignore the error.
	191	+ }
189	192
190	193	return lib;
191	194	},

Collapse file

‎test/parallel/test-cli-eval.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-cli-eval.js

+9Lines changed: 9 additions & 0 deletions

Original file line number	Diff line number	Diff line change
@@ -354,3 +354,12 @@ child.exec(
354	354	common.mustSucceed((stdout) => {
355	355	assert.match(stdout, /^number/);
356	356	}));
	357	+
	358	+ // Regression test for https://github.com/nodejs/node/issues/45336
	359	+ child.execFile(process.execPath,
	360	+ ['-p',
	361	+ 'Object.defineProperty(global, "fs", { configurable: false });' +
	362	+ 'fs === require("node:fs")'],
	363	+ common.mustSucceed((stdout) => {
	364	+ assert.match(stdout, /^true/);
	365	+ }));

0 commit comments

Comments

0 (0)