Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit e7f604f

Browse filesBrowse files
bfarias-godaddyBridgeAR
bfarias-godaddy
authored and
BridgeAR
committed
esm: remove proxy for builtin exports
PR-URL: #29737 Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Jan Krems <jan.krems@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Minwoo Jung <minwoo@nodesource.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent ae8b2b4 commit e7f604f
Copy full SHA for e7f604f

File tree

Expand file treeCollapse file tree

6 files changed

+107
-69
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

6 files changed

+107
-69
lines changed
Open diff view settings
Collapse file

‎doc/api/esm.md‎

Copy file name to clipboardExpand all lines: doc/api/esm.md
+8-4Lines changed: 8 additions & 4 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -555,10 +555,11 @@ cjs === 'cjs'; // true
555555
556556
## Builtin modules
557557
558-
Builtin modules will provide named exports of their public API, as well as a
559-
default export which can be used for, among other things, modifying the named
560-
exports. Named exports of builtin modules are updated when the corresponding
561-
exports property is accessed, redefined, or deleted.
558+
Builtin modules will provide named exports of their public API. A
559+
default export is also provided which is the value of the CommonJS exports.
560+
The default export can be used for, among other things, modifying the named
561+
exports. Named exports of builtin modules are updated only by calling
562+
[`module.syncBuiltinESMExports()`][].
562563
563564
```js
564565
import EventEmitter from 'events';
@@ -578,8 +579,10 @@ readFile('./foo.txt', (err, source) => {
578579
579580
```js
580581
import fs, { readFileSync } from 'fs';
582+
import { syncBuiltinESMExports } from 'module';
581583

582584
fs.readFileSync = () => Buffer.from('Hello, ESM');
585+
syncBuiltinESMExports();
583586

584587
fs.readFileSync === readFileSync;
585588
```
@@ -1008,6 +1011,7 @@ success!
10081011
[`import.meta.url`]: #esm_import_meta
10091012
[`import`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import
10101013
[`module.createRequire()`]: modules.html#modules_module_createrequire_filename
1014+
[`module.syncBuiltinESMExports()`]: modules.html#modules_module_syncbuiltinesmexports
10111015
[dynamic instantiate hook]: #esm_dynamic_instantiate_hook
10121016
[package exports]: #esm_package_exports
10131017
[special scheme]: https://url.spec.whatwg.org/#special-scheme
Collapse file

‎doc/api/modules.md‎

Copy file name to clipboardExpand all lines: doc/api/modules.md
+30Lines changed: 30 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -984,6 +984,36 @@ const requireUtil = createRequireFromPath('../src/utils/');
984984
requireUtil('./some-tool');
985985
```
986986
987+
### module.syncBuiltinESMExports()
988+
<!-- YAML
989+
added: REPLACEME
990+
-->
991+
992+
The `module.syncBuiltinESMExports()` method updates all the live bindings for
993+
builtin ES Modules to match the properties of the CommonJS exports. It does
994+
not add or remove exported names from the ES Modules.
995+
996+
```js
997+
const fs = require('fs');
998+
const { syncBuiltinESMExports } = require('module');
999+
1000+
fs.readFile = null;
1001+
1002+
delete fs.readFileSync;
1003+
1004+
fs.newAPI = function newAPI() {
1005+
// ...
1006+
};
1007+
1008+
syncBuiltinESMExports();
1009+
1010+
import('fs').then((esmFS) => {
1011+
assert.strictEqual(esmFS.readFile, null);
1012+
assert.strictEqual('readFileSync' in fs, true);
1013+
assert.strictEqual(esmFS.newAPI, undefined);
1014+
});
1015+
```
1016+
9871017
[GLOBAL_FOLDERS]: #modules_loading_from_the_global_folders
9881018
[`Error`]: errors.html#errors_class_error
9891019
[`__dirname`]: #modules_dirname
Collapse file

‎lib/internal/bootstrap/loaders.js‎

Copy file name to clipboardExpand all lines: lib/internal/bootstrap/loaders.js
+37-55Lines changed: 37 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ function NativeModule(id) {
151151
this.id = id;
152152
this.exports = {};
153153
this.reflect = undefined;
154+
this.esmFacade = undefined;
154155
this.exportKeys = undefined;
155156
this.loaded = false;
156157
this.loading = false;
@@ -211,15 +212,19 @@ function requireWithFallbackInDeps(request) {
211212
}
212213

213214
// This is exposed for public loaders
214-
NativeModule.prototype.compileForPublicLoader = function(needToProxify) {
215+
NativeModule.prototype.compileForPublicLoader = function(needToSyncExports) {
215216
if (!this.canBeRequiredByUsers) {
216217
// No code because this is an assertion against bugs
217218
// eslint-disable-next-line no-restricted-syntax
218219
throw new Error(`Should not compile ${this.id} for public use`);
219220
}
220221
this.compile();
221-
if (needToProxify && !this.exportKeys) {
222-
this.proxifyExports();
222+
if (needToSyncExports) {
223+
if (!this.exportKeys) {
224+
this.exportKeys = Object.keys(this.exports);
225+
}
226+
this.getESMFacade();
227+
this.syncExports();
223228
}
224229
return this.exports;
225230
};
@@ -230,61 +235,38 @@ const getOwn = (target, property, receiver) => {
230235
undefined;
231236
};
232237

238+
NativeModule.prototype.getURL = function() {
239+
return `node:${this.id}`;
240+
};
241+
242+
NativeModule.prototype.getESMFacade = function() {
243+
if (this.esmFacade) return this.esmFacade;
244+
const createDynamicModule = nativeModuleRequire(
245+
'internal/modules/esm/create_dynamic_module');
246+
const url = this.getURL();
247+
return this.esmFacade = createDynamicModule(
248+
[], [...this.exportKeys, 'default'], url, (reflect) => {
249+
this.reflect = reflect;
250+
this.syncExports();
251+
reflect.exports.default.set(this.exports);
252+
});
253+
};
254+
233255
// Provide named exports for all builtin libraries so that the libraries
234256
// may be imported in a nicer way for ESM users. The default export is left
235-
// as the entire namespace (module.exports) and wrapped in a proxy such
236-
// that APMs and other behavior are still left intact.
237-
NativeModule.prototype.proxifyExports = function() {
238-
this.exportKeys = Object.keys(this.exports);
239-
240-
const update = (property, value) => {
241-
if (this.reflect !== undefined &&
242-
ObjectPrototype.hasOwnProperty(this.reflect.exports, property))
243-
this.reflect.exports[property].set(value);
244-
};
245-
246-
const handler = {
247-
__proto__: null,
248-
defineProperty: (target, prop, descriptor) => {
249-
// Use `Object.defineProperty` instead of `Reflect.defineProperty`
250-
// to throw the appropriate error if something goes wrong.
251-
Object.defineProperty(target, prop, descriptor);
252-
if (typeof descriptor.get === 'function' &&
253-
!Reflect.has(handler, 'get')) {
254-
handler.get = (target, prop, receiver) => {
255-
const value = Reflect.get(target, prop, receiver);
256-
if (ObjectPrototype.hasOwnProperty(target, prop))
257-
update(prop, value);
258-
return value;
259-
};
260-
}
261-
update(prop, getOwn(target, prop));
262-
return true;
263-
},
264-
deleteProperty: (target, prop) => {
265-
if (Reflect.deleteProperty(target, prop)) {
266-
update(prop, undefined);
267-
return true;
268-
}
269-
return false;
270-
},
271-
set: (target, prop, value, receiver) => {
272-
const descriptor = Reflect.getOwnPropertyDescriptor(target, prop);
273-
if (Reflect.set(target, prop, value, receiver)) {
274-
if (descriptor && typeof descriptor.set === 'function') {
275-
for (const key of this.exportKeys) {
276-
update(key, getOwn(target, key, receiver));
277-
}
278-
} else {
279-
update(prop, getOwn(target, prop, receiver));
280-
}
281-
return true;
282-
}
283-
return false;
257+
// as the entire namespace (module.exports) and updates when this function is
258+
// called so that APMs and other behavior are supported.
259+
NativeModule.prototype.syncExports = function() {
260+
const names = this.exportKeys;
261+
if (this.reflect) {
262+
for (let i = 0; i < names.length; i++) {
263+
const exportName = names[i];
264+
if (exportName === 'default') continue;
265+
this.reflect.exports[exportName].set(
266+
getOwn(this.exports, exportName, this.exports)
267+
);
284268
}
285-
};
286-
287-
this.exports = new Proxy(this.exports, handler);
269+
}
288270
};
289271

290272
NativeModule.prototype.compile = function() {
Collapse file

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

Copy file name to clipboardExpand all lines: lib/internal/modules/cjs/loader.js
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1130,6 +1130,14 @@ Module._preloadModules = function(requests) {
11301130
parent.require(requests[n]);
11311131
};
11321132

1133+
Module.syncBuiltinESMExports = function syncBuiltinESMExports() {
1134+
for (const mod of NativeModule.map.values()) {
1135+
if (mod.canBeRequiredByUsers) {
1136+
mod.syncExports();
1137+
}
1138+
}
1139+
};
1140+
11331141
// Backwards compatibility
11341142
Module.Module = Module;
11351143

Collapse file

‎lib/internal/modules/esm/translators.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/translators.js
+2-8Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -129,14 +129,8 @@ translators.set('builtin', async function builtinStrategy(url) {
129129
if (!module) {
130130
throw new ERR_UNKNOWN_BUILTIN_MODULE(id);
131131
}
132-
return createDynamicModule(
133-
[], [...module.exportKeys, 'default'], url, (reflect) => {
134-
debug(`Loading BuiltinModule ${url}`);
135-
module.reflect = reflect;
136-
for (const key of module.exportKeys)
137-
reflect.exports[key].set(module.exports[key]);
138-
reflect.exports.default.set(module.exports);
139-
});
132+
debug(`Loading BuiltinModule ${url}`);
133+
return module.getESMFacade();
140134
});
141135

142136
// Strategy for loading a JSON file
Collapse file

‎test/es-module/test-esm-live-binding.mjs‎

Copy file name to clipboardExpand all lines: test/es-module/test-esm-live-binding.mjs
+22-2Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Flags: --experimental-modules
22
import '../common/index.mjs';
33
import assert from 'assert';
4+
import { syncBuiltinESMExports } from 'module';
45

56
import fs, { readFile, readFileSync } from 'fs';
67
import events, { defaultMaxListeners } from 'events';
@@ -14,10 +15,12 @@ const s = Symbol();
1415
const fn = () => s;
1516

1617
Reflect.deleteProperty(fs, 'readFile');
18+
syncBuiltinESMExports();
1719

1820
assert.deepStrictEqual([fs.readFile, readFile], [undefined, undefined]);
1921

2022
fs.readFile = fn;
23+
syncBuiltinESMExports();
2124

2225
assert.deepStrictEqual([fs.readFile(), readFile()], [s, s]);
2326

@@ -26,10 +29,12 @@ Reflect.defineProperty(fs, 'readFile', {
2629
configurable: true,
2730
writable: true,
2831
});
32+
syncBuiltinESMExports();
2933

3034
assert.deepStrictEqual([fs.readFile(), readFile()], [s, s]);
3135

3236
Reflect.deleteProperty(fs, 'readFile');
37+
syncBuiltinESMExports();
3338

3439
assert.deepStrictEqual([fs.readFile, readFile], [undefined, undefined]);
3540

@@ -39,10 +44,14 @@ Reflect.defineProperty(fs, 'readFile', {
3944
get() { return count; },
4045
configurable: true,
4146
});
47+
syncBuiltinESMExports();
48+
49+
assert.deepStrictEqual([readFile, fs.readFile], [0, 0]);
4250

4351
count++;
52+
syncBuiltinESMExports();
4453

45-
assert.deepStrictEqual([readFile, fs.readFile, readFile], [0, 1, 1]);
54+
assert.deepStrictEqual([fs.readFile, readFile], [1, 1]);
4655

4756
let otherValue;
4857

@@ -62,6 +71,7 @@ Reflect.defineProperty(fs, 'readFileSync', {
6271
});
6372

6473
fs.readFile = 2;
74+
syncBuiltinESMExports();
6575

6676
assert.deepStrictEqual([readFile, readFileSync], [undefined, 2]);
6777

@@ -86,17 +96,23 @@ Reflect.defineProperty(Function.prototype, 'defaultMaxListeners', {
8696
});
8797
},
8898
});
99+
syncBuiltinESMExports();
89100

90101
assert.strictEqual(defaultMaxListeners, originDefaultMaxListeners);
91102
assert.strictEqual(events.defaultMaxListeners, originDefaultMaxListeners);
92103

93-
assert.strictEqual(++events.defaultMaxListeners,
104+
events.defaultMaxListeners += 1;
105+
106+
assert.strictEqual(events.defaultMaxListeners,
94107
originDefaultMaxListeners + 1);
95108

109+
syncBuiltinESMExports();
110+
96111
assert.strictEqual(defaultMaxListeners, originDefaultMaxListeners + 1);
97112
assert.strictEqual(Function.prototype.defaultMaxListeners, 1);
98113

99114
Function.prototype.defaultMaxListeners = 'foo';
115+
syncBuiltinESMExports();
100116

101117
assert.strictEqual(Function.prototype.defaultMaxListeners, 'foo');
102118
assert.strictEqual(events.defaultMaxListeners, originDefaultMaxListeners + 1);
@@ -117,16 +133,19 @@ const p = {
117133
};
118134

119135
util.__proto__ = p; // eslint-disable-line no-proto
136+
syncBuiltinESMExports();
120137

121138
assert.strictEqual(util.foo, 1);
122139

123140
util.foo = 'bar';
141+
syncBuiltinESMExports();
124142

125143
assert.strictEqual(count, 1);
126144
assert.strictEqual(util.foo, 'bar');
127145
assert.strictEqual(p.foo, 2);
128146

129147
p.foo = 'foo';
148+
syncBuiltinESMExports();
130149

131150
assert.strictEqual(p.foo, 'foo');
132151

@@ -135,6 +154,7 @@ util.__proto__ = utilProto; // eslint-disable-line no-proto
135154

136155
Reflect.deleteProperty(util, 'foo');
137156
Reflect.deleteProperty(Function.prototype, 'defaultMaxListeners');
157+
syncBuiltinESMExports();
138158

139159
assert.throws(
140160
() => Object.defineProperty(events, 'defaultMaxListeners', { value: 3 }),

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.