From 3aabe355fbad86dc2f5825926de93091053b05df 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: `import { readFile } from 'fs'` instead of importing the entire namespace, `import fs from 'fs'`, and calling `fs.readFile`. the default export is left as the entire namespace (module.exports) --- doc/api/esm.md | 30 ++++- lib/internal/bootstrap/loaders.js | 123 ++++++++++++++++++ lib/internal/bootstrap/node.js | 2 +- .../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 | 42 ++++++ test/es-module/test-esm-namespace.mjs | 10 +- test/fixtures/es-module-loaders/js-loader.mjs | 7 +- 9 files changed, 222 insertions(+), 13 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 4090e545fdb54b..11ec39d1e539c7 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -95,8 +95,26 @@ 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. + +```js +import EventEmitter from 'events'; +const e = new EventEmitter(); +``` + +```js +import { readFile } from 'fs'; +readFile('./foo.txt', (err, body) => { if (err) { console.error(err); } else { @@ -105,6 +123,14 @@ fs.readFile('./foo.txt', (err, body) => { }); ``` +```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..34abeee8596e81 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -95,6 +95,8 @@ this.filename = `${id}.js`; this.id = id; this.exports = {}; + this.reflect = undefined; + this.exportKeys = undefined; this.loaded = false; this.loading = false; } @@ -193,6 +195,40 @@ '\n});' ]; + const { isProxy } = internalBinding('types'); + const { + apply: ReflectApply, + has: ReflectHas, + get: ReflectGet, + set: ReflectSet, + defineProperty: ReflectDefineProperty, + deleteProperty: ReflectDeleteProperty, + getOwnPropertyDescriptor: ReflectGetOwnPropertyDescriptor, + } = Reflect; + const { + toString: ObjectToString, + } = Object.prototype; + let isNative; + { + const { toString } = Function.prototype; + const re = toString.call(toString) + .replace(/[\\^$.*+?()[\]{}|]/g, '\\$&') + .replace(/toString|(function ).*?(?=\\\()/g, '$1.*?'); + const nativeRegExp = new RegExp(`^${re}$`); + isNative = (fn) => { + if (typeof fn === 'function') { + try { + if (nativeRegExp.test(toString.call(fn))) { + const { name } = fn; + if (typeof name !== 'string' || !/^bound /.test(name)) + return !isProxy(fn); + } + } catch (e) {} + } + return false; + }; + } + NativeModule.prototype.compile = function() { let source = NativeModule.getSource(this.id); source = NativeModule.wrap(source); @@ -208,6 +244,93 @@ NativeModule.require; fn(this.exports, requireFn, this, process); + if (config.experimentalModules) { + this.exportKeys = Object.keys(this.exports); + + const update = (property, value) => { + if (this.reflect !== undefined && this.exportKeys.includes(property)) + this.reflect.exports[property].set(value); + }; + + const methodWrapMap = new WeakMap(); + + const wrap = (target, name, value) => { + if (typeof value !== 'function' || !isNative(value)) { + return value; + } + + if (methodWrapMap.has(value)) + return methodWrapMap.get(value); + + const p = new Proxy(value, { + apply: (t, thisArg, args) => { + if (thisArg === proxy || (this.reflect !== undefined && + this.reflect.namespace !== undefined && + thisArg === this.reflect.namespace)) { + thisArg = target; + } + return ReflectApply(t, thisArg, args); + }, + __proto__: null, + }); + + methodWrapMap.set(value, p); + + return p; + }; + + const proxy = new Proxy(this.exports, { + set: (target, prop, value, receiver) => { + if (receiver === proxy || (this.reflect !== undefined && + this.reflect.namespace !== undefined && + receiver === this.reflect.namespace)) + receiver = target; + if (ReflectSet(target, prop, value, receiver)) { + update(prop, ReflectGet(target, prop, receiver)); + return true; + } + return false; + }, + defineProperty: (target, prop, descriptor) => { + if (ReflectDefineProperty(target, prop, descriptor)) { + update(prop, ReflectGet(target, prop)); + return true; + } + return false; + }, + deleteProperty: (target, prop) => { + if (ReflectDeleteProperty(target, prop)) { + update(prop, undefined); + return true; + } + return false; + }, + getOwnPropertyDescriptor: (target, prop) => { + const descriptor = ReflectGetOwnPropertyDescriptor(target, prop); + if (descriptor && ReflectHas(descriptor, 'value')) + descriptor.value = wrap(target, prop, descriptor.value); + return descriptor; + }, + get: (target, prop, receiver) => { + if (receiver === proxy || (this.reflect !== undefined && + this.reflect.namespace !== undefined && + receiver === this.reflect.namespace)) { + receiver = target; + } + const value = ReflectGet(target, prop, receiver); + if (prop === Symbol.toStringTag && + typeof target !== 'function' && + typeof value !== 'string') { + const toStringTag = ObjectToString.call(target).slice(8, -1); + return toStringTag === 'Object' ? value : toStringTag; + } + return wrap(target, prop, value); + }, + __proto__: null, + }); + this.exports = proxy; + } + this.loaded = true; } finally { this.loading = false; diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 5993cc71f7d404..42835e235890a3 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -403,7 +403,7 @@ // If global console has the same method as inspector console, // then wrap these two methods into one. Native wrapper will preserve // the original stack. - wrappedConsole[key] = consoleCall.bind(wrappedConsole, + wrappedConsole[key] = consoleCall.bind(null, originalConsole[key], wrappedConsole[key], config); 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..73c7f5982dd8a3 --- /dev/null +++ b/test/es-module/test-esm-live-binding.mjs @@ -0,0 +1,42 @@ +// Flags: --experimental-modules + +import '../common'; +import assert from 'assert'; + +import fs, { readFile } from 'fs'; + +const s = Symbol(); +const fn = () => s; + +delete fs.readFile; +assert.strictEqual(fs.readFile, undefined); +assert.strictEqual(readFile, undefined); + +fs.readFile = fn; + +assert.strictEqual(fs.readFile(), s); +assert.strictEqual(readFile(), s); + +Reflect.deleteProperty(fs, 'readFile'); + +Reflect.defineProperty(fs, 'readFile', { + value: fn, + configurable: true, + writable: true, +}); + +assert.strictEqual(fs.readFile(), s); +assert.strictEqual(readFile(), s); + +Reflect.deleteProperty(fs, 'readFile'); +assert.strictEqual(fs.readFile, undefined); +assert.strictEqual(readFile, undefined); + +Reflect.defineProperty(fs, 'readFile', { + get() { return fn; }, + set() {}, + configurable: true, +}); + +assert.strictEqual(fs.readFile(), s); +assert.strictEqual(readFile(), s); 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'