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>

Author: bfarias-godaddy

doc/api/esm.md

@@ -555,10 +555,11 @@ cjs === 'cjs'; // true
 
 ## Builtin modules
 
-Builtin modules will provide named exports of their public API, as well as a
-default export which can be used for, among other things, modifying the named
-exports. Named exports of builtin modules are updated when the corresponding
-exports property is accessed, redefined, or deleted.
+Builtin modules will provide named exports of their public API. A
+default export is also provided which is the value of the CommonJS exports.
+The default export can be used for, among other things, modifying the named
+exports. Named exports of builtin modules are updated only by calling
+[`module.syncBuiltinESMExports()`][].
 
 ```js
 importEventEmitterfrom'events';
@@ -578,8 +579,10 @@ readFile('./foo.txt', (err, source) => {
 
 ```js
 importfs, { readFileSync } from'fs';
+import { syncBuiltinESMExports } from'module';
 
 fs.readFileSync= () =>Buffer.from('Hello, ESM');
+syncBuiltinESMExports();
 
 fs.readFileSync=== readFileSync;
 ```
@@ -1008,6 +1011,7 @@ success!
 [`import.meta.url`]: #esm_import_meta
 [`import`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import
 [`module.createRequire()`]: modules.html#modules_module_createrequire_filename
+[`module.syncBuiltinESMExports()`]: modules.html#modules_module_syncbuiltinesmexports
 [dynamic instantiate hook]: #esm_dynamic_instantiate_hook
 [package exports]: #esm_package_exports
 [special scheme]: https://url.spec.whatwg.org/#special-scheme

doc/api/modules.md

@@ -984,6 +984,36 @@ const requireUtil = createRequireFromPath('../src/utils/');
 requireUtil('./some-tool');
 ```
 
+### module.syncBuiltinESMExports()
+<!-- YAML
+added: REPLACEME
+-->
+
+The `module.syncBuiltinESMExports()` method updates all the live bindings for
+builtin ES Modules to match the properties of the CommonJS exports. It does
+not add or remove exported names from the ES Modules.
+
+```js
+constfs=require('fs');
+const { syncBuiltinESMExports } =require('module');
+
+fs.readFile=null;
+
+deletefs.readFileSync;
+
+fs.newAPI=functionnewAPI() {
+// ...
+};
+
+syncBuiltinESMExports();
+
+import('fs').then((esmFS) => {
+assert.strictEqual(esmFS.readFile, null);
+assert.strictEqual('readFileSync'in fs, true);
+assert.strictEqual(esmFS.newAPI, undefined);
+});
+```
+
 [GLOBAL_FOLDERS]: #modules_loading_from_the_global_folders
 [`Error`]: errors.html#errors_class_error
 [`__dirname`]: #modules_dirname

lib/internal/bootstrap/loaders.js

@@ -151,6 +151,7 @@ function NativeModule(id) {
 this.id=id;
 this.exports={};
 this.reflect=undefined;
+this.esmFacade=undefined;
 this.exportKeys=undefined;
 this.loaded=false;
 this.loading=false;
@@ -211,15 +212,19 @@ function requireWithFallbackInDeps(request) {
 }
 
 // This is exposed for public loaders
-NativeModule.prototype.compileForPublicLoader=function(needToProxify){
+NativeModule.prototype.compileForPublicLoader=function(needToSyncExports){
 if(!this.canBeRequiredByUsers){
 // No code because this is an assertion against bugs
 // eslint-disable-next-line no-restricted-syntax
 thrownewError(`Should not compile ${this.id} for public use`);
 }
 this.compile();
-if(needToProxify&&!this.exportKeys){
-this.proxifyExports();
+if(needToSyncExports){
+if(!this.exportKeys){
+this.exportKeys=Object.keys(this.exports);
+}
+this.getESMFacade();
+this.syncExports();
 }
 returnthis.exports;
 };
@@ -230,61 +235,38 @@ const getOwn = (target, property, receiver) => {
 undefined;
 };
 
+NativeModule.prototype.getURL=function(){
+return`node:${this.id}`;
+};
+
+NativeModule.prototype.getESMFacade=function(){
+if(this.esmFacade)returnthis.esmFacade;
+constcreateDynamicModule=nativeModuleRequire(
+'internal/modules/esm/create_dynamic_module');
+consturl=this.getURL();
+returnthis.esmFacade=createDynamicModule(
+[],[...this.exportKeys,'default'],url,(reflect)=>{
+this.reflect=reflect;
+this.syncExports();
+reflect.exports.default.set(this.exports);
+});
+};
+
 // Provide named exports for all builtin libraries so that the libraries
 // may be imported in a nicer way for ESM users. The default export is left
-// as the entire namespace (module.exports) and wrapped in a proxy such
-// that APMs and other behavior are still left intact.
-NativeModule.prototype.proxifyExports=function(){
-this.exportKeys=Object.keys(this.exports);
-
-constupdate=(property,value)=>{
-if(this.reflect!==undefined&&
-ObjectPrototype.hasOwnProperty(this.reflect.exports,property))
-this.reflect.exports[property].set(value);
-};
-
-consthandler={
-__proto__: null,
-defineProperty: (target,prop,descriptor)=>{
-// Use `Object.defineProperty` instead of `Reflect.defineProperty`
-// to throw the appropriate error if something goes wrong.
-Object.defineProperty(target,prop,descriptor);
-if(typeofdescriptor.get==='function'&&
-!Reflect.has(handler,'get')){
-handler.get=(target,prop,receiver)=>{
-constvalue=Reflect.get(target,prop,receiver);
-if(ObjectPrototype.hasOwnProperty(target,prop))
-update(prop,value);
-returnvalue;
-};
-}
-update(prop,getOwn(target,prop));
-returntrue;
-},
-deleteProperty: (target,prop)=>{
-if(Reflect.deleteProperty(target,prop)){
-update(prop,undefined);
-returntrue;
-}
-returnfalse;
-},
-set: (target,prop,value,receiver)=>{
-constdescriptor=Reflect.getOwnPropertyDescriptor(target,prop);
-if(Reflect.set(target,prop,value,receiver)){
-if(descriptor&&typeofdescriptor.set==='function'){
-for(constkeyofthis.exportKeys){
-update(key,getOwn(target,key,receiver));
-}
-}else{
-update(prop,getOwn(target,prop,receiver));
-}
-returntrue;
-}
-returnfalse;
+// as the entire namespace (module.exports) and updates when this function is
+// called so that APMs and other behavior are supported.
+NativeModule.prototype.syncExports=function(){
+constnames=this.exportKeys;
+if(this.reflect){
+for(leti=0;i<names.length;i++){
+constexportName=names[i];
+if(exportName==='default')continue;
+this.reflect.exports[exportName].set(
+getOwn(this.exports,exportName,this.exports)
+);
 }
-};
-
-this.exports=newProxy(this.exports,handler);
+}
 };
 
 NativeModule.prototype.compile=function(){

lib/internal/modules/cjs/loader.js

@@ -1130,6 +1130,14 @@ Module._preloadModules = function(requests) {
 parent.require(requests[n]);
 };
 
+Module.syncBuiltinESMExports=functionsyncBuiltinESMExports(){
+for(constmodofNativeModule.map.values()){
+if(mod.canBeRequiredByUsers){
+mod.syncExports();
+}
+}
+};
+
 // Backwards compatibility
 Module.Module=Module;
 

lib/internal/modules/esm/translators.js

@@ -129,14 +129,8 @@ translators.set('builtin', async function builtinStrategy(url) {
 if(!module){
 thrownewERR_UNKNOWN_BUILTIN_MODULE(id);
 }
-returncreateDynamicModule(
-[],[...module.exportKeys,'default'],url,(reflect)=>{
-debug(`Loading BuiltinModule ${url}`);
-module.reflect=reflect;
-for(constkeyofmodule.exportKeys)
-reflect.exports[key].set(module.exports[key]);
-reflect.exports.default.set(module.exports);
-});
+debug(`Loading BuiltinModule ${url}`);
+returnmodule.getESMFacade();
 });
 
 // Strategy for loading a JSON file

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

@@ -1,6 +1,7 @@
 // Flags: --experimental-modules
 import'../common/index.mjs';
 importassertfrom'assert';
+import{syncBuiltinESMExports}from'module';
 
 importfs,{readFile,readFileSync}from'fs';
 importevents,{defaultMaxListeners}from'events';
@@ -14,10 +15,12 @@ const s = Symbol();
 constfn=()=>s;
 
 Reflect.deleteProperty(fs,'readFile');
+syncBuiltinESMExports();
 
 assert.deepStrictEqual([fs.readFile,readFile],[undefined,undefined]);
 
 fs.readFile=fn;
+syncBuiltinESMExports();
 
 assert.deepStrictEqual([fs.readFile(),readFile()],[s,s]);
 
@@ -26,10 +29,12 @@ Reflect.defineProperty(fs, 'readFile', {
 configurable: true,
 writable: true,
 });
+syncBuiltinESMExports();
 
 assert.deepStrictEqual([fs.readFile(),readFile()],[s,s]);
 
 Reflect.deleteProperty(fs,'readFile');
+syncBuiltinESMExports();
 
 assert.deepStrictEqual([fs.readFile,readFile],[undefined,undefined]);
 
@@ -39,10 +44,14 @@ Reflect.defineProperty(fs, 'readFile', {
 get(){returncount;},
 configurable: true,
 });
+syncBuiltinESMExports();
+
+assert.deepStrictEqual([readFile,fs.readFile],[0,0]);
 
 count++;
+syncBuiltinESMExports();
 
-assert.deepStrictEqual([readFile,fs.readFile,readFile],[0,1,1]);
+assert.deepStrictEqual([fs.readFile,readFile],[1,1]);
 
 letotherValue;
 
@@ -62,6 +71,7 @@ Reflect.defineProperty(fs, 'readFileSync', {
 });
 
 fs.readFile=2;
+syncBuiltinESMExports();
 
 assert.deepStrictEqual([readFile,readFileSync],[undefined,2]);
 
@@ -86,17 +96,23 @@ Reflect.defineProperty(Function.prototype, 'defaultMaxListeners', {
 });
 },
 });
+syncBuiltinESMExports();
 
 assert.strictEqual(defaultMaxListeners,originDefaultMaxListeners);
 assert.strictEqual(events.defaultMaxListeners,originDefaultMaxListeners);
 
-assert.strictEqual(++events.defaultMaxListeners,
+events.defaultMaxListeners+=1;
+
+assert.strictEqual(events.defaultMaxListeners,
 originDefaultMaxListeners+1);
 
+syncBuiltinESMExports();
+
 assert.strictEqual(defaultMaxListeners,originDefaultMaxListeners+1);
 assert.strictEqual(Function.prototype.defaultMaxListeners,1);
 
 Function.prototype.defaultMaxListeners='foo';
+syncBuiltinESMExports();
 
 assert.strictEqual(Function.prototype.defaultMaxListeners,'foo');
 assert.strictEqual(events.defaultMaxListeners,originDefaultMaxListeners+1);
@@ -117,16 +133,19 @@ const p = {
 };
 
 util.__proto__=p;// eslint-disable-line no-proto
+syncBuiltinESMExports();
 
 assert.strictEqual(util.foo,1);
 
 util.foo='bar';
+syncBuiltinESMExports();
 
 assert.strictEqual(count,1);
 assert.strictEqual(util.foo,'bar');
 assert.strictEqual(p.foo,2);
 
 p.foo='foo';
+syncBuiltinESMExports();
 
 assert.strictEqual(p.foo,'foo');
 
@@ -135,6 +154,7 @@ util.__proto__ = utilProto; // eslint-disable-line no-proto
 
 Reflect.deleteProperty(util,'foo');
 Reflect.deleteProperty(Function.prototype,'defaultMaxListeners');
+syncBuiltinESMExports();
 
 assert.throws(
 ()=>Object.defineProperty(events,'defaultMaxListeners',{value: 3}),