From f074612b744fecfd1f79fd05d7f10fe9a1487e5a Mon Sep 17 00:00:00 2001 From: Gus Caplan Date: Sun, 22 Apr 2018 18:56:42 -0500 Subject: [PATCH] esm: provide named exports for builtin libs 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. PR-URL: https://github.com/nodejs/node/pull/20403 Reviewed-By: Bradley Farias Reviewed-By: Guy Bedford Reviewed-By: Jan Krems --- doc/api/esm.md | 33 +++- lib/internal/bootstrap/loaders.js | 84 +++++++++- .../modules/esm/create_dynamic_module.js | 3 +- lib/internal/modules/esm/translators.js | 17 ++- test/es-module/test-esm-dynamic-import.js | 1 + test/es-module/test-esm-live-binding.mjs | 143 ++++++++++++++++++ test/es-module/test-esm-namespace.mjs | 10 +- test/fixtures/es-module-loaders/js-loader.mjs | 7 +- 8 files changed, 282 insertions(+), 16 deletions(-) create mode 100644 test/es-module/test-esm-live-binding.mjs diff --git a/doc/api/esm.md b/doc/api/esm.md index a1e3cb149ab7d8..459f87771815cc 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -95,16 +95,43 @@ When loaded via `import` these modules will provide a single `default` export representing the value of `module.exports` at the time they finished evaluating. ```js -import fs from 'fs'; -fs.readFile('./foo.txt', (err, body) => { +// foo.js +module.exports = { one: 1 }; + +// bar.js +import foo from './foo.js'; +foo.one === 1; // true +``` + +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. + +```js +import EventEmitter from 'events'; +const e = new EventEmitter(); +``` + +```js +import { readFile } from 'fs'; +readFile('./foo.txt', (err, source) => { if (err) { console.error(err); } else { - console.log(body); + console.log(source); } }); ``` +```js +import fs, { readFileSync } from 'fs'; + +fs.readFileSync = () => Buffer.from('Hello, ESM'); + +fs.readFileSync === readFileSync; +``` + ## Loader hooks diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index 828370f6eadd01..7dc44d5bbe28fb 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -41,10 +41,26 @@ (function bootstrapInternalLoaders(process, getBinding, getLinkedBinding, getInternalBinding) { + const { + apply: ReflectApply, + deleteProperty: ReflectDeleteProperty, + get: ReflectGet, + getOwnPropertyDescriptor: ReflectGetOwnPropertyDescriptor, + has: ReflectHas, + set: ReflectSet, + } = Reflect; + const { + prototype: { + hasOwnProperty: ObjectHasOwnProperty, + }, + create: ObjectCreate, + defineProperty: ObjectDefineProperty, + keys: ObjectKeys, + } = Object; // Set up process.moduleLoadList const moduleLoadList = []; - Object.defineProperty(process, 'moduleLoadList', { + ObjectDefineProperty(process, 'moduleLoadList', { value: moduleLoadList, configurable: true, enumerable: true, @@ -53,7 +69,7 @@ // Set up process.binding() and process._linkedBinding() { - const bindingObj = Object.create(null); + const bindingObj = ObjectCreate(null); process.binding = function binding(module) { module = String(module); @@ -77,7 +93,7 @@ // Set up internalBinding() in the closure let internalBinding; { - const bindingObj = Object.create(null); + const bindingObj = ObjectCreate(null); internalBinding = function internalBinding(module) { let mod = bindingObj[module]; if (typeof mod !== 'object') { @@ -95,6 +111,8 @@ this.filename = `${id}.js`; this.id = id; this.exports = {}; + this.reflect = undefined; + this.exportKeys = undefined; this.loaded = false; this.loading = false; } @@ -193,6 +211,12 @@ '\n});' ]; + const getOwn = (target, property, receiver) => { + return ReflectApply(ObjectHasOwnProperty, target, [property]) ? + ReflectGet(target, property, receiver) : + undefined; + }; + NativeModule.prototype.compile = function() { let source = NativeModule.getSource(this.id); source = NativeModule.wrap(source); @@ -208,6 +232,60 @@ NativeModule.require; fn(this.exports, requireFn, this, process); + if (config.experimentalModules && !NativeModule.isInternal(this.id)) { + this.exportKeys = ObjectKeys(this.exports); + + const update = (property, value) => { + if (this.reflect !== undefined && + ReflectApply(ObjectHasOwnProperty, + this.reflect.exports, [property])) + this.reflect.exports[property].set(value); + }; + + const handler = { + __proto__: null, + defineProperty: (target, prop, descriptor) => { + // Use `Object.defineProperty` instead of `Reflect.defineProperty` + // to throw the appropriate error if something goes wrong. + ObjectDefineProperty(target, prop, descriptor); + if (typeof descriptor.get === 'function' && + !ReflectHas(handler, 'get')) { + handler.get = (target, prop, receiver) => { + const value = ReflectGet(target, prop, receiver); + if (ReflectApply(ObjectHasOwnProperty, target, [prop])) + update(prop, value); + return value; + }; + } + update(prop, getOwn(target, prop)); + return true; + }, + deleteProperty: (target, prop) => { + if (ReflectDeleteProperty(target, prop)) { + update(prop, undefined); + return true; + } + return false; + }, + set: (target, prop, value, receiver) => { + const descriptor = ReflectGetOwnPropertyDescriptor(target, prop); + if (ReflectSet(target, prop, value, receiver)) { + if (descriptor && typeof descriptor.set === 'function') { + for (const key of this.exportKeys) { + update(key, getOwn(target, key, receiver)); + } + } else { + update(prop, getOwn(target, prop, receiver)); + } + return true; + } + return false; + } + }; + + this.exports = new Proxy(this.exports, handler); + } + this.loaded = true; } finally { this.loading = false; diff --git a/lib/internal/modules/esm/create_dynamic_module.js b/lib/internal/modules/esm/create_dynamic_module.js index e0c97263631432..c01844feb8b654 100644 --- a/lib/internal/modules/esm/create_dynamic_module.js +++ b/lib/internal/modules/esm/create_dynamic_module.js @@ -52,9 +52,10 @@ const createDynamicModule = (exports, url = '', evaluate) => { const module = new ModuleWrap(reexports, `${url}`); module.link(async () => reflectiveModule); module.instantiate(); + reflect.namespace = module.namespace(); return { module, - reflect + reflect, }; }; diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index d181647c4505e8..4ed6b2c3b023a9 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -59,11 +59,18 @@ translators.set('cjs', async (url) => { // through normal resolution translators.set('builtin', async (url) => { debug(`Translating BuiltinModule ${url}`); - return createDynamicModule(['default'], url, (reflect) => { - debug(`Loading BuiltinModule ${url}`); - const exports = NativeModule.require(url.slice(5)); - reflect.exports.default.set(exports); - }); + // slice 'node:' scheme + const id = url.slice(5); + NativeModule.require(id); + const module = NativeModule.getCached(id); + return createDynamicModule( + [...module.exportKeys, 'default'], url, (reflect) => { + debug(`Loading BuiltinModule ${url}`); + module.reflect = reflect; + for (const key of module.exportKeys) + reflect.exports[key].set(module.exports[key]); + reflect.exports.default.set(module.exports); + }); }); // Strategy for loading a node native module diff --git a/test/es-module/test-esm-dynamic-import.js b/test/es-module/test-esm-dynamic-import.js index c6daa95f9a6b98..91757172880f67 100644 --- a/test/es-module/test-esm-dynamic-import.js +++ b/test/es-module/test-esm-dynamic-import.js @@ -52,6 +52,7 @@ function expectFsNamespace(result) { Promise.resolve(result) .then(common.mustCall(ns => { assert.strictEqual(typeof ns.default.writeFile, 'function'); + assert.strictEqual(typeof ns.writeFile, 'function'); })); } diff --git a/test/es-module/test-esm-live-binding.mjs b/test/es-module/test-esm-live-binding.mjs new file mode 100644 index 00000000000000..d151e004dff4b2 --- /dev/null +++ b/test/es-module/test-esm-live-binding.mjs @@ -0,0 +1,143 @@ +// Flags: --experimental-modules + +import '../common'; +import assert from 'assert'; + +import fs, { readFile, readFileSync } from 'fs'; +import events, { defaultMaxListeners } from 'events'; +import util from 'util'; + +const readFileDescriptor = Reflect.getOwnPropertyDescriptor(fs, 'readFile'); +const readFileSyncDescriptor = + Reflect.getOwnPropertyDescriptor(fs, 'readFileSync'); + +const s = Symbol(); +const fn = () => s; + +Reflect.deleteProperty(fs, 'readFile'); + +assert.deepStrictEqual([fs.readFile, readFile], [undefined, undefined]); + +fs.readFile = fn; + +assert.deepStrictEqual([fs.readFile(), readFile()], [s, s]); + +Reflect.defineProperty(fs, 'readFile', { + value: fn, + configurable: true, + writable: true, +}); + +assert.deepStrictEqual([fs.readFile(), readFile()], [s, s]); + +Reflect.deleteProperty(fs, 'readFile'); + +assert.deepStrictEqual([fs.readFile, readFile], [undefined, undefined]); + +let count = 0; + +Reflect.defineProperty(fs, 'readFile', { + get() { return count; }, + configurable: true, +}); + +count++; + +assert.deepStrictEqual([readFile, fs.readFile, readFile], [0, 1, 1]); + +let otherValue; + +Reflect.defineProperty(fs, 'readFile', { // eslint-disable-line accessor-pairs + set(value) { + Reflect.deleteProperty(fs, 'readFile'); + otherValue = value; + }, + configurable: true, +}); + +Reflect.defineProperty(fs, 'readFileSync', { + get() { + return otherValue; + }, + configurable: true, +}); + +fs.readFile = 2; + +assert.deepStrictEqual([readFile, readFileSync], [undefined, 2]); + +Reflect.defineProperty(fs, 'readFile', readFileDescriptor); +Reflect.defineProperty(fs, 'readFileSync', readFileSyncDescriptor); + +const originDefaultMaxListeners = events.defaultMaxListeners; +const utilProto = util.__proto__; // eslint-disable-line no-proto + +count = 0; + +Reflect.defineProperty(Function.prototype, 'defaultMaxListeners', { + configurable: true, + enumerable: true, + get: function() { return ++count; }, + set: function(v) { + Reflect.defineProperty(this, 'defaultMaxListeners', { + configurable: true, + enumerable: true, + writable: true, + value: v, + }); + }, +}); + +assert.strictEqual(defaultMaxListeners, originDefaultMaxListeners); +assert.strictEqual(events.defaultMaxListeners, originDefaultMaxListeners); + +assert.strictEqual(++events.defaultMaxListeners, + originDefaultMaxListeners + 1); + +assert.strictEqual(defaultMaxListeners, originDefaultMaxListeners + 1); +assert.strictEqual(Function.prototype.defaultMaxListeners, 1); + +Function.prototype.defaultMaxListeners = 'foo'; + +assert.strictEqual(Function.prototype.defaultMaxListeners, 'foo'); +assert.strictEqual(events.defaultMaxListeners, originDefaultMaxListeners + 1); +assert.strictEqual(defaultMaxListeners, originDefaultMaxListeners + 1); + +count = 0; + +const p = { + get foo() { return ++count; }, + set foo(v) { + Reflect.defineProperty(this, 'foo', { + configurable: true, + enumerable: true, + writable: true, + value: v, + }); + }, +}; + +util.__proto__ = p; // eslint-disable-line no-proto + +assert.strictEqual(util.foo, 1); + +util.foo = 'bar'; + +assert.strictEqual(count, 1); +assert.strictEqual(util.foo, 'bar'); +assert.strictEqual(p.foo, 2); + +p.foo = 'foo'; + +assert.strictEqual(p.foo, 'foo'); + +events.defaultMaxListeners = originDefaultMaxListeners; +util.__proto__ = utilProto; // eslint-disable-line no-proto + +Reflect.deleteProperty(util, 'foo'); +Reflect.deleteProperty(Function.prototype, 'defaultMaxListeners'); + +assert.throws( + () => Object.defineProperty(events, 'defaultMaxListeners', { value: 3 }), + /TypeError: Cannot redefine/ +); diff --git a/test/es-module/test-esm-namespace.mjs b/test/es-module/test-esm-namespace.mjs index 04845e2b3e4da0..dcd159f6c8729a 100644 --- a/test/es-module/test-esm-namespace.mjs +++ b/test/es-module/test-esm-namespace.mjs @@ -2,5 +2,13 @@ import '../common'; import * as fs from 'fs'; import assert from 'assert'; +import Module from 'module'; -assert.deepStrictEqual(Object.keys(fs), ['default']); +const keys = Object.entries( + Object.getOwnPropertyDescriptors(new Module().require('fs'))) + .filter(([name, d]) => d.enumerable) + .map(([name]) => name) + .concat('default') + .sort(); + +assert.deepStrictEqual(Object.keys(fs).sort(), keys); diff --git a/test/fixtures/es-module-loaders/js-loader.mjs b/test/fixtures/es-module-loaders/js-loader.mjs index 2173b0b503ef45..9fa6b9eed4d029 100644 --- a/test/fixtures/es-module-loaders/js-loader.mjs +++ b/test/fixtures/es-module-loaders/js-loader.mjs @@ -1,10 +1,11 @@ -import _url from 'url'; +import { URL } from 'url'; + const builtins = new Set( Object.keys(process.binding('natives')).filter(str => /^(?!(?:internal|node|v8)\/)/.test(str)) ) -const baseURL = new _url.URL('file://'); +const baseURL = new URL('file://'); baseURL.pathname = process.cwd() + '/'; export function resolve (specifier, base = baseURL) { @@ -15,7 +16,7 @@ export function resolve (specifier, base = baseURL) { }; } // load all dependencies as esm, regardless of file extension - const url = new _url.URL(specifier, base).href; + const url = new URL(specifier, base).href; return { url, format: 'esm'