From 8c9e3a9dfb16f52cc9ac1949901f13e79c62a24f Mon Sep 17 00:00:00 2001 From: Jan Krems Date: Thu, 21 May 2020 09:40:29 -0700 Subject: [PATCH] module: remove dynamicInstantiate loader hook The dynamicInstantiate loader hook requires that the hooks run in the same global scope as the code being loaded. We don't want to commit to this being true in the future. It stops us from sharing hooks between multiple worker threads or isolating loader hook from the application code. Using `getSource` and `getGlobalPreloadCode` the same use cases should be covered. PR-URL: https://github.com/nodejs/node/pull/33501 Reviewed-By: Geoffrey Booth Reviewed-By: Guy Bedford Reviewed-By: Bradley Farias --- doc/api/errors.md | 14 ---- doc/api/esm.md | 35 ---------- lib/internal/errors.js | 3 - lib/internal/modules/esm/loader.js | 41 ++---------- ...oader-missing-dynamic-instantiate-hook.mjs | 8 --- test/es-module/test-esm-named-exports.mjs | 3 +- .../builtin-named-exports-loader.mjs | 67 +++++++++++++++---- 7 files changed, 60 insertions(+), 111 deletions(-) delete mode 100644 test/es-module/test-esm-loader-missing-dynamic-instantiate-hook.mjs diff --git a/doc/api/errors.md b/doc/api/errors.md index 80b5f44bc59cbe..c322ddb1b5a51b 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1570,14 +1570,6 @@ strict compliance with the API specification (which in some cases may accept `func(undefined)` and `func()` are treated identically, and the [`ERR_INVALID_ARG_TYPE`][] error code may be used instead. - -### `ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK` - -> Stability: 1 - Experimental - -An [ES Module][] loader hook specified `format: 'dynamic'` but did not provide -a `dynamicInstantiate` hook. - ### `ERR_MISSING_OPTION` @@ -2519,12 +2511,6 @@ while trying to read and parse it. The `--entry-type=...` flag is not compatible with the Node.js REPL. - -#### `ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK` - -Used when an [ES Module][] loader hook specifies `format: 'dynamic'` but does -not provide a `dynamicInstantiate` hook. - #### `ERR_FEATURE_UNAVAILABLE_ON_PLATFORM` diff --git a/doc/api/esm.md b/doc/api/esm.md index f8a7595a721873..38885d7a0b243e 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -1204,7 +1204,6 @@ of the following: | --- | --- | --- | | `'builtin'` | Load a Node.js builtin module | Not applicable | | `'commonjs'` | Load a Node.js CommonJS module | Not applicable | -| `'dynamic'` | Use a [dynamic instantiate hook][] | Not applicable | | `'json'` | Load a JSON file | { [ArrayBuffer][], [string][], [TypedArray][] } | | `'module'` | Load an ES module | { [ArrayBuffer][], [string][], [TypedArray][] } | | `'wasm'` | Load a WebAssembly module | { [ArrayBuffer][], [string][], [TypedArray][] } | @@ -1345,38 +1344,6 @@ const require = createRequire(process.cwd() + '/'); } ``` -#### dynamicInstantiate hook - -> Note: The loaders API is being redesigned. This hook may disappear or its -> signature may change. Do not rely on the API described below. - -To create a custom dynamic module that doesn't correspond to one of the -existing `format` interpretations, the `dynamicInstantiate` hook can be used. -This hook is called only for modules that return `format: 'dynamic'` from -the [`getFormat` hook][]. - -```js -/** - * @param {string} url - * @returns {object} response - * @returns {array} response.exports - * @returns {function} response.execute - */ -export async function dynamicInstantiate(url) { - return { - exports: ['customExportName'], - execute: (exports) => { - // Get and set functions provided for pre-allocated export names - exports.customExportName.set('value'); - } - }; -} -``` - -With the list of module exports provided upfront, the `execute` function will -then be called at the exact point of module evaluation order for that module -in the import tree. - ### Examples The various loader hooks can be used together to accomplish wide-ranging @@ -1846,7 +1813,6 @@ success! [`data:` URLs]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/Data_URIs [`esm`]: https://github.com/standard-things/esm#readme [`export`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/export -[`getFormat` hook]: #esm_code_getformat_code_hook [`import()`]: #esm_import_expressions [`import.meta.url`]: #esm_import_meta [`import`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import @@ -1859,7 +1825,6 @@ success! [TypedArray]: http://www.ecma-international.org/ecma-262/6.0/#sec-typedarray-objects [Uint8Array]: http://www.ecma-international.org/ecma-262/6.0/#sec-uint8array [`util.TextDecoder`]: util.html#util_class_util_textdecoder -[dynamic instantiate hook]: #esm_code_dynamicinstantiate_code_hook [import an ES or CommonJS module for its side effects only]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import#Import_a_module_for_its_side_effects_only [special scheme]: https://url.spec.whatwg.org/#special-scheme [the full specifier path]: #esm_mandatory_file_extensions diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 8bd39eaaad082f..b1c434e25c3f1d 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1258,9 +1258,6 @@ E('ERR_MISSING_ARGS', } return `${msg} must be specified`; }, TypeError); -E('ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK', - 'The ES Module loader may not return a format of \'dynamic\' when no ' + - 'dynamicInstantiate function was provided', Error); E('ERR_MISSING_OPTION', '%s is required', TypeError); E('ERR_MODULE_NOT_FOUND', (path, base, type = 'package') => { return `Cannot find ${type} '${path}' imported from ${base}`; diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 135c541813d49c..de08845a820d77 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -11,7 +11,6 @@ const { ERR_INVALID_RETURN_PROPERTY, ERR_INVALID_RETURN_PROPERTY_VALUE, ERR_INVALID_RETURN_VALUE, - ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK, ERR_UNKNOWN_MODULE_FORMAT } = require('internal/errors').codes; const { URL, pathToFileURL } = require('internal/url'); @@ -28,14 +27,10 @@ const { defaultGetSource } = require( 'internal/modules/esm/get_source'); const { defaultTransformSource } = require( 'internal/modules/esm/transform_source'); -const createDynamicModule = require( - 'internal/modules/esm/create_dynamic_module'); const { translators } = require( 'internal/modules/esm/translators'); const { getOptionValue } = require('internal/options'); -const debug = require('internal/util/debuglog').debuglog('esm'); - /* A Loader instance is used as the main entry point for loading ES modules. * Currently, this is a singleton -- there is only one used for loading * the main module and everything in its dependency graph. */ @@ -68,8 +63,6 @@ class Loader { // This hook is called after the module is resolved but before a translator // is chosen to load it; the format returned by this function is the name // of a translator. - // If `.format` on the returned value is 'dynamic', .dynamicInstantiate - // will be used as described below. this._getFormat = defaultGetFormat; // This hook is called just before the source code of an ES module file // is loaded. @@ -77,14 +70,6 @@ class Loader { // This hook is called just after the source code of an ES module file // is loaded, but before anything is done with the string. this._transformSource = defaultTransformSource; - // This hook is only called when getFormat is 'dynamic' and - // has the signature - // (url : string) -> Promise<{ exports: { ... }, execute: function }> - // Where `exports` is an object whose property names define the exported - // names of the generated module. `execute` is a function that receives - // an object with the same keys as `exports`, whose values are get/set - // functions for the actual exported values. - this._dynamicInstantiate = undefined; // The index for assigning unique URLs to anonymous module evaluation this.evalIndex = 0; } @@ -138,7 +123,6 @@ class Loader { } if (this._resolve === defaultResolve && - format !== 'dynamic' && !url.startsWith('file:') && !url.startsWith('data:') ) { @@ -193,8 +177,8 @@ class Loader { if (resolve !== undefined) this._resolve = FunctionPrototypeBind(resolve, null); if (dynamicInstantiate !== undefined) { - this._dynamicInstantiate = - FunctionPrototypeBind(dynamicInstantiate, null); + process.emitWarning( + 'The dynamicInstantiate loader hook has been removed.'); } if (getFormat !== undefined) { this._getFormat = FunctionPrototypeBind(getFormat, null); @@ -248,25 +232,10 @@ class Loader { if (job !== undefined) return job; - let loaderInstance; - if (format === 'dynamic') { - if (typeof this._dynamicInstantiate !== 'function') - throw new ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK(); - - loaderInstance = async (url) => { - debug(`Translating dynamic ${url}`); - const { exports, execute } = await this._dynamicInstantiate(url); - return createDynamicModule([], exports, url, (reflect) => { - debug(`Loading dynamic ${url}`); - execute(reflect.exports); - }).module; - }; - } else { - if (!translators.has(format)) - throw new ERR_UNKNOWN_MODULE_FORMAT(format); + if (!translators.has(format)) + throw new ERR_UNKNOWN_MODULE_FORMAT(format); - loaderInstance = translators.get(format); - } + const loaderInstance = translators.get(format); const inspectBrk = parentURL === undefined && format === 'module' && getOptionValue('--inspect-brk'); diff --git a/test/es-module/test-esm-loader-missing-dynamic-instantiate-hook.mjs b/test/es-module/test-esm-loader-missing-dynamic-instantiate-hook.mjs deleted file mode 100644 index 62781c37d48240..00000000000000 --- a/test/es-module/test-esm-loader-missing-dynamic-instantiate-hook.mjs +++ /dev/null @@ -1,8 +0,0 @@ -// Flags: --experimental-loader ./test/fixtures/es-module-loaders/missing-dynamic-instantiate-hook.mjs -import { expectsError } from '../common/index.mjs'; - -import('test').catch(expectsError({ - code: 'ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK', - message: 'The ES Module loader may not return a format of \'dynamic\' ' + - 'when no dynamicInstantiate function was provided' -})); diff --git a/test/es-module/test-esm-named-exports.mjs b/test/es-module/test-esm-named-exports.mjs index 7d8d1080082401..ce8599e68b1bf5 100644 --- a/test/es-module/test-esm-named-exports.mjs +++ b/test/es-module/test-esm-named-exports.mjs @@ -1,8 +1,9 @@ // Flags: --experimental-loader ./test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs import '../common/index.mjs'; -import { readFile } from 'fs'; +import { readFile, __fromLoader } from 'fs'; import assert from 'assert'; import ok from '../fixtures/es-modules/test-esm-ok.mjs'; assert(ok); assert(readFile); +assert(__fromLoader); diff --git a/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs b/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs index 9f1bc24560b87a..f476c676cdea5b 100644 --- a/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs +++ b/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs @@ -1,23 +1,62 @@ import module from 'module'; -export function getFormat(url, context, defaultGetFormat) { - if (module.builtinModules.includes(url)) { +const GET_BUILTIN = `$__get_builtin_hole_${Date.now()}`; + +export function getGlobalPreloadCode() { + return `Object.defineProperty(globalThis, ${JSON.stringify(GET_BUILTIN)}, { + value: (builtinName) => { + return getBuiltin(builtinName); + }, + enumerable: false, + configurable: false, +}); +`; +} + +export function resolve(specifier, context, defaultResolve) { + const def = defaultResolve(specifier, context); + if (def.url.startsWith('nodejs:')) { + return { + url: `custom-${def.url}`, + }; + } + return def; +} + +export function getSource(url, context, defaultGetSource) { + if (url.startsWith('custom-nodejs:')) { + const urlObj = new URL(url); return { - format: 'dynamic' + source: generateBuiltinModule(urlObj.pathname), + format: 'module', }; } + return defaultGetSource(url, context); +} + +export function getFormat(url, context, defaultGetFormat) { + if (url.startsWith('custom-nodejs:')) { + return { format: 'module' }; + } return defaultGetFormat(url, context, defaultGetFormat); } -export function dynamicInstantiate(url) { - const builtinInstance = module._load(url); - const builtinExports = ['default', ...Object.keys(builtinInstance)]; - return { - exports: builtinExports, - execute: exports => { - for (let name of builtinExports) - exports[name].set(builtinInstance[name]); - exports.default.set(builtinInstance); - } - }; +function generateBuiltinModule(builtinName) { + const builtinInstance = module._load(builtinName); + const builtinExports = [ + ...Object.keys(builtinInstance), + ]; + return `\ +const $builtinInstance = ${GET_BUILTIN}(${JSON.stringify(builtinName)}); + +export const __fromLoader = true; + +export default $builtinInstance; + +${ + builtinExports + .map(name => `export const ${name} = $builtinInstance.${name};`) + .join('\n') +} +`; }