From 6e9e023724cfb006efa3a914e40658e04799ed69 Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Wed, 21 Feb 2018 14:10:12 -0600 Subject: [PATCH 1/9] Allow for multiple --loader flags --- doc/api/esm.md | 85 ++++++++++++------- lib/internal/process/modules.js | 51 +++++++++-- src/node.cc | 4 +- src/node_config.cc | 19 +++-- src/node_internals.h | 2 +- test/es-module/esm-export-url.mjs | 2 + test/es-module/esm-log-import-url.mjs | 3 + test/es-module/loader-add-hash.mjs | 16 ++++ test/es-module/loader-remove-params.mjs | 17 ++++ test/es-module/test-esm-loader-multiple.js | 75 ++++++++++++++++ .../builtin-named-exports-loader.mjs | 42 +++++---- .../es-module-loaders/example-loader.mjs | 47 +++++----- test/fixtures/es-module-loaders/js-loader.mjs | 26 +++--- .../es-module-loaders/loader-shared-dep.mjs | 10 ++- .../es-module-loaders/loader-with-dep.mjs | 10 ++- .../not-found-assert-loader.mjs | 36 ++++---- 16 files changed, 325 insertions(+), 120 deletions(-) create mode 100644 test/es-module/esm-export-url.mjs create mode 100644 test/es-module/esm-log-import-url.mjs create mode 100644 test/es-module/loader-add-hash.mjs create mode 100644 test/es-module/loader-remove-params.mjs create mode 100644 test/es-module/test-esm-loader-multiple.js diff --git a/doc/api/esm.md b/doc/api/esm.md index adde6a199fd068..431569dbcc3409 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -106,11 +106,21 @@ fs.readFile('./foo.txt', (err, body) => { To customize the default module resolution, loader hooks can optionally be -provided via a `--loader ./loader-name.mjs` argument to Node. +provided via a `--loader ./loader-name.mjs` argument to Node. This argument +can be passed multiple times to compose loaders like +`--loader ./loader-coverage.mjs --loader ./loader-mocking.mjs`. The last loader +will be used for module loading and must explicitly call to the parent loader +in order to provide compose behavior. When hooks are used they only apply to ES module loading and not to any CommonJS modules loaded. +All loaders are created by invoking the default export of their module as a +function. The parameters given to the the function is a single object with +properties to call the `resolve` and `dynamicInstantiate` hooks of the parent +loader. The default loader has a `resolve` hook and `null` for the value +`dynamicInstantiate`. + ### Resolve hook The resolve hook returns the resolved file URL and module format for a @@ -120,12 +130,15 @@ given module specifier and parent file URL: const baseURL = new URL('file://'); baseURL.pathname = `${process.cwd()}/`; -export async function resolve(specifier, - parentModuleURL = baseURL, - defaultResolver) { +export default function (parent) { return { - url: new URL(specifier, parentModuleURL).href, - format: 'esm' + async resolve(specifier, + parentModuleURL = baseURL) { + return { + url: new URL(specifier, parentModuleURL).href, + format: 'esm' + }; + } }; } ``` @@ -164,28 +177,32 @@ const JS_EXTENSIONS = new Set(['.js', '.mjs']); const baseURL = new URL('file://'); baseURL.pathname = `${process.cwd()}/`; -export function resolve(specifier, parentModuleURL = baseURL, defaultResolve) { - if (builtins.includes(specifier)) { - return { - url: specifier, - format: 'builtin' - }; - } - if (/^\.{0,2}[/]/.test(specifier) !== true && !specifier.startsWith('file:')) { - // For node_modules support: - // return defaultResolve(specifier, parentModuleURL); - throw new Error( - `imports must begin with '/', './', or '../'; '${specifier}' does not`); - } - const resolved = new URL(specifier, parentModuleURL); - const ext = path.extname(resolved.pathname); - if (!JS_EXTENSIONS.has(ext)) { - throw new Error( - `Cannot load file with non-JavaScript file extension ${ext}.`); - } +export default function (parent) { return { - url: resolved.href, - format: 'esm' + resolve(specifier, parentModuleURL = baseURL) { + if (builtins.includes(specifier)) { + return { + url: specifier, + format: 'builtin' + }; + } + if (/^\.{0,2}[/]/.test(specifier) !== true && !specifier.startsWith('file:')) { + // For node_modules support: + // return parent.resolve(specifier, parentModuleURL); + throw new Error( + `imports must begin with '/', './', or '../'; '${specifier}' does not`); + } + const resolved = new URL(specifier, parentModuleURL); + const ext = path.extname(resolved.pathname); + if (!JS_EXTENSIONS.has(ext)) { + throw new Error( + `Cannot load file with non-JavaScript file extension ${ext}.`); + } + return { + url: resolved.href, + format: 'esm' + }; + } }; } ``` @@ -207,12 +224,16 @@ This hook is called only for modules that return `format: "dynamic"` from the `resolve` hook. ```js -export async function dynamicInstantiate(url) { +export default function (parent) { return { - exports: ['customExportName'], - execute: (exports) => { - // get and set functions provided for pre-allocated export names - exports.customExportName.set('value'); + dynamicInstantiate(url) { + return { + exports: ['customExportName'], + execute: (exports) => { + // get and set functions provided for pre-allocated export names + exports.customExportName.set('value'); + } + }; } }; } diff --git a/lib/internal/process/modules.js b/lib/internal/process/modules.js index f89278ddaa2d52..09722b80c19afb 100644 --- a/lib/internal/process/modules.js +++ b/lib/internal/process/modules.js @@ -31,15 +31,50 @@ exports.ESMLoader = undefined; exports.setup = function() { setInitializeImportMetaObjectCallback(initializeImportMetaObject); - let ESMLoader = new Loader(); + const ESMLoader = new Loader(); const loaderPromise = (async () => { - const userLoader = process.binding('config').userLoader; - if (userLoader) { - const hooks = await ESMLoader.import( - userLoader, getURLFromFilePath(`${process.cwd()}/`).href); - ESMLoader = new Loader(); - ESMLoader.hook(hooks); - exports.ESMLoader = ESMLoader; + const { userLoaders } = process.binding('config'); + if (userLoaders) { + let previous = { + resolve: (url, referrer) => { + return require('internal/loader/DefaultResolve')(url, referrer); + }, + dynamicInstantiate: null + }; + for (var i = 0; i < userLoaders.length; i++) { + const loaderSpecifier = userLoaders[i]; + const { default: factory } = await ESMLoader.import(loaderSpecifier); + const parent = previous; + previous = factory({ + __proto__: null, + resolve: parent.resolve ? Object.setPrototypeOf( + async (url, referrer) => { + const ret = await parent.resolve(url, referrer); + return { + __proto__: null, + url: `${ret.url}`, + format: `${ret.format}`, + }; + }, + null + ) : null, + dynamicInstantiate: parent.dynamicInstantiate ? Object.setPrototypeOf( + async (url) => { + const ret = await parent.dynamicInstantiate(url); + return { + __proto__: null, + exports: ret.exports, + execute: ret.execute + }; + }, + null + ) : null + }); + } + ESMLoader.hook({ + resolve: previous.resolve !== undefined ? previous.resolve.bind(previous) : undefined, + dynamicInstantiate: previous.dynamicInstantiate !== undefined ? previous.dynamicInstantiate.bind(previous) : undefined + }); } return ESMLoader; })(); diff --git a/src/node.cc b/src/node.cc index 64de859bc6a278..6fc1c5f5f7e5d1 100644 --- a/src/node.cc +++ b/src/node.cc @@ -246,7 +246,7 @@ bool config_experimental_vm_modules = false; // Set in node.cc by ParseArgs when --loader is used. // Used in node_config.cc to set a constant on process.binding('config') // that is used by lib/internal/bootstrap_node.js -std::string config_userland_loader; // NOLINT(runtime/string) +std::vector config_userland_loaders; // Set by ParseArgs when --pending-deprecation or NODE_PENDING_DEPRECATION // is used. @@ -3745,7 +3745,7 @@ static void ParseArgs(int* argc, exit(9); } args_consumed += 1; - config_userland_loader = module; + config_userland_loaders.push_back(module); } else if (strcmp(arg, "--prof-process") == 0) { prof_process = true; short_circuit = true; diff --git a/src/node_config.cc b/src/node_config.cc index cac551ad2c410a..80ffb7b78425ca 100644 --- a/src/node_config.cc +++ b/src/node_config.cc @@ -71,13 +71,22 @@ static void InitConfig(Local target, if (config_experimental_modules) { READONLY_BOOLEAN_PROPERTY("experimentalModules"); - if (!config_userland_loader.empty()) { + if (!config_userland_loaders.empty()) { + auto loaders = v8::Array::New(isolate, config_userland_loaders.size()); + for (size_t i = 0; i < config_userland_loaders.size(); i++) { + auto item = config_userland_loaders[i]; + loaders->Set( + i, + String::NewFromUtf8(isolate, + item.data(), + v8::NewStringType::kNormal).ToLocalChecked()); + } + + loaders->SetIntegrityLevel(context, v8::IntegrityLevel::kFrozen); target->DefineOwnProperty( context, - FIXED_ONE_BYTE_STRING(isolate, "userLoader"), - String::NewFromUtf8(isolate, - config_userland_loader.data(), - v8::NewStringType::kNormal).ToLocalChecked(), + FIXED_ONE_BYTE_STRING(isolate, "userLoaders"), + loaders, ReadOnly).FromJust(); } } diff --git a/src/node_internals.h b/src/node_internals.h index 140826bb7c5ebd..c6a3fb0facea8a 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -182,7 +182,7 @@ extern bool config_experimental_vm_modules; // Set in node.cc by ParseArgs when --loader is used. // Used in node_config.cc to set a constant on process.binding('config') // that is used by lib/internal/bootstrap_node.js -extern std::string config_userland_loader; +extern std::vector config_userland_loaders; // Set in node.cc by ParseArgs when --expose-internals or --expose_internals is // used. diff --git a/test/es-module/esm-export-url.mjs b/test/es-module/esm-export-url.mjs new file mode 100644 index 00000000000000..242a27aabd4514 --- /dev/null +++ b/test/es-module/esm-export-url.mjs @@ -0,0 +1,2 @@ +/* eslint-disable node-core/required-modules */ +export default import.meta.url; diff --git a/test/es-module/esm-log-import-url.mjs b/test/es-module/esm-log-import-url.mjs new file mode 100644 index 00000000000000..ad8062068323da --- /dev/null +++ b/test/es-module/esm-log-import-url.mjs @@ -0,0 +1,3 @@ +/* eslint-disable node-core/required-modules */ +import url from './esm-export-url.mjs?x=y'; +process.stdout.write(url); diff --git a/test/es-module/loader-add-hash.mjs b/test/es-module/loader-add-hash.mjs new file mode 100644 index 00000000000000..9387f89a9997f6 --- /dev/null +++ b/test/es-module/loader-add-hash.mjs @@ -0,0 +1,16 @@ +/* eslint-disable node-core/required-modules */ +import url from 'url'; + +export default ({ + resolve: parentResolve +}) => ({ + async resolve(specifier, parentModuleURL) { + const parentResolved = await parentResolve(specifier, parentModuleURL); + const request = new url.URL(parentResolved.url); + request.hash = '#hash'; + return { + url: request.href, + format: parentResolved.format + }; + } +}); diff --git a/test/es-module/loader-remove-params.mjs b/test/es-module/loader-remove-params.mjs new file mode 100644 index 00000000000000..f88d52d2d5197a --- /dev/null +++ b/test/es-module/loader-remove-params.mjs @@ -0,0 +1,17 @@ +/* eslint-disable node-core/required-modules */ +import url from 'url'; + +export default ({ + resolve: parentResolve +}) => ({ + async resolve(specifier, parentModuleURL) { + const parentResolved = await parentResolve(specifier, parentModuleURL); + const request = new url.URL(parentResolved.url); + request.search = ''; + request.hash = ''; + return { + url: request.href, + format: parentResolved.format + }; + } +}); diff --git a/test/es-module/test-esm-loader-multiple.js b/test/es-module/test-esm-loader-multiple.js new file mode 100644 index 00000000000000..d38acfdb48b090 --- /dev/null +++ b/test/es-module/test-esm-loader-multiple.js @@ -0,0 +1,75 @@ +// Flags: --experimental-modules +'use strict'; + +require('../common'); +const { spawnSync } = require('child_process'); +const assert = require('assert'); +const path = require('path'); +const url = require('url'); + +const loader_add_hash = path.join(__dirname, 'loader-add-hash.mjs'); +const loader_remove_params = path.join(__dirname, 'loader-remove-params.mjs'); +const logger = path.join(__dirname, 'esm-log-import-url.mjs'); +const dependency = path.join(__dirname, 'esm-export-url.mjs'); + +{ + const result = spawnSync( + process.execPath, + [ + '--experimental-modules', + '--loader', + loader_remove_params, + '--loader', + loader_add_hash, + logger, + ], + { + stdio: 'pipe', + } + ); + assert.strictEqual(result.status, 0); + const dependencyResolved = new url.URL(`${result.stdout}`); + assert.strictEqual(dependencyResolved.pathname, dependency); + assert.strictEqual(dependencyResolved.hash, '#hash'); + assert.strictEqual(dependencyResolved.search, ''); +} + +{ + const result = spawnSync( + process.execPath, + [ + '--experimental-modules', + '--loader', + loader_add_hash, + '--loader', + loader_remove_params, + logger, + ], + { + stdio: 'pipe', + } + ); + assert.strictEqual(result.status, 0); + const dependencyResolved = new url.URL(`${result.stdout}`); + assert.strictEqual(dependencyResolved.pathname, dependency); + assert.strictEqual(dependencyResolved.hash, ''); + assert.strictEqual(dependencyResolved.search, ''); +} + +{ + const result = spawnSync( + process.execPath, + [ + '--experimental-modules', + logger, + ], + { + stdio: 'pipe', + } + ); + assert.strictEqual(result.status, 0); + const dependencyResolved = new url.URL(`${result.stdout}`); + assert.strictEqual(dependencyResolved.pathname, dependency); + assert.strictEqual(dependencyResolved.hash, ''); + assert.strictEqual(dependencyResolved.search, '?x=y'); +} 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 28ccd6ecf2076f..830b2536b3567b 100644 --- a/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs +++ b/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs @@ -5,25 +5,31 @@ const builtins = new Set( /^(?!(?:internal|node|v8)\/)/.test(str)) ); -export function dynamicInstantiate(url) { - const builtinInstance = module._load(url.substr(5)); - const builtinExports = ['default', ...Object.keys(builtinInstance)]; +export default ({ + resolve: parentResolve +}) => { return { - exports: builtinExports, - execute: exports => { - for (let name of builtinExports) - exports[name].set(builtinInstance[name]); - exports.default.set(builtinInstance); + dynamicInstantiate(url) { + const builtinInstance = module._load(url.substr(5)); + 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); + } + }; + }, + + resolve(specifier, base) { + if (builtins.has(specifier)) { + return { + url: `node:${specifier}`, + format: 'dynamic' + }; + } + return parentResolve(specifier, base); } }; } - -export function resolve(specifier, base, defaultResolver) { - if (builtins.has(specifier)) { - return { - url: `node:${specifier}`, - format: 'dynamic' - }; - } - return defaultResolver(specifier, base); -} diff --git a/test/fixtures/es-module-loaders/example-loader.mjs b/test/fixtures/es-module-loaders/example-loader.mjs index acb4486edc1288..531013cd5ec7ab 100644 --- a/test/fixtures/es-module-loaders/example-loader.mjs +++ b/test/fixtures/es-module-loaders/example-loader.mjs @@ -11,27 +11,32 @@ const JS_EXTENSIONS = new Set(['.js', '.mjs']); const baseURL = new url.URL('file://'); baseURL.pathname = process.cwd() + '/'; -export function resolve(specifier, parentModuleURL = baseURL /*, defaultResolve */) { - if (builtins.has(specifier)) { - return { - url: specifier, - format: 'builtin' - }; - } - if (/^\.{0,2}[/]/.test(specifier) !== true && !specifier.startsWith('file:')) { - // For node_modules support: - // return defaultResolve(specifier, parentModuleURL); - throw new Error( - `imports must begin with '/', './', or '../'; '${specifier}' does not`); - } - const resolved = new url.URL(specifier, parentModuleURL); - const ext = path.extname(resolved.pathname); - if (!JS_EXTENSIONS.has(ext)) { - throw new Error( - `Cannot load file with non-JavaScript file extension ${ext}.`); - } + +export default ({ resolve: parentResolve }) => { return { - url: resolved.href, - format: 'esm' + resolve(specifier, parentModuleURL = baseURL) { + if (builtins.has(specifier)) { + return { + url: specifier, + format: 'builtin' + }; + } + if (/^\.{0,2}[/]/.test(specifier) !== true && !specifier.startsWith('file:')) { + // For node_modules support: + // return defaultResolve(specifier, parentModuleURL); + throw new Error( + `imports must begin with '/', './', or '../'; '${specifier}' does not`); + } + const resolved = new url.URL(specifier, parentModuleURL); + const ext = path.extname(resolved.pathname); + if (!JS_EXTENSIONS.has(ext)) { + throw new Error( + `Cannot load file with non-JavaScript file extension ${ext}.`); + } + return { + url: resolved.href, + format: 'esm' + }; + } }; } diff --git a/test/fixtures/es-module-loaders/js-loader.mjs b/test/fixtures/es-module-loaders/js-loader.mjs index 2173b0b503ef45..9ab589b0e6cf88 100644 --- a/test/fixtures/es-module-loaders/js-loader.mjs +++ b/test/fixtures/es-module-loaders/js-loader.mjs @@ -7,17 +7,21 @@ const builtins = new Set( const baseURL = new _url.URL('file://'); baseURL.pathname = process.cwd() + '/'; -export function resolve (specifier, base = baseURL) { - if (builtins.has(specifier)) { - return { - url: specifier, - format: 'builtin' - }; - } - // load all dependencies as esm, regardless of file extension - const url = new _url.URL(specifier, base).href; +export default ({ resolve: parentResolve }) => { return { - url, - format: 'esm' + resolve(specifier, base = baseURL) { + if (builtins.has(specifier)) { + return { + url: specifier, + format: 'builtin' + }; + } + // load all dependencies as esm, regardless of file extension + const url = new _url.URL(specifier, base).href; + return { + url, + format: 'esm' + }; + } }; } diff --git a/test/fixtures/es-module-loaders/loader-shared-dep.mjs b/test/fixtures/es-module-loaders/loader-shared-dep.mjs index 1a19e4c8927527..4410363609a494 100644 --- a/test/fixtures/es-module-loaders/loader-shared-dep.mjs +++ b/test/fixtures/es-module-loaders/loader-shared-dep.mjs @@ -1,7 +1,11 @@ import dep from './loader-dep.js'; import assert from 'assert'; -export function resolve(specifier, base, defaultResolve) { - assert.strictEqual(dep.format, 'esm'); - return defaultResolve(specifier, base); +export default ({ resolve: parentResolve }) => { + return { + resolve(specifier, base, defaultResolve) { + assert.strictEqual(dep.format, 'esm'); + return parentResolve(specifier, base); + } + }; } diff --git a/test/fixtures/es-module-loaders/loader-with-dep.mjs b/test/fixtures/es-module-loaders/loader-with-dep.mjs index 944e6e438c5cbd..6443ccd7fddbc9 100644 --- a/test/fixtures/es-module-loaders/loader-with-dep.mjs +++ b/test/fixtures/es-module-loaders/loader-with-dep.mjs @@ -1,7 +1,11 @@ import dep from './loader-dep.js'; -export function resolve (specifier, base, defaultResolve) { +export default ({ resolve: parentResolve }) => { return { - url: defaultResolve(specifier, base).url, - format: dep.format + async resolve(specifier, base, defaultResolve) { + return { + url: (await parentResolve(specifier, base)).url, + format: dep.format + }; + } }; } diff --git a/test/fixtures/es-module-loaders/not-found-assert-loader.mjs b/test/fixtures/es-module-loaders/not-found-assert-loader.mjs index d15f294fe674bb..4a1482cc1e6186 100644 --- a/test/fixtures/es-module-loaders/not-found-assert-loader.mjs +++ b/test/fixtures/es-module-loaders/not-found-assert-loader.mjs @@ -3,20 +3,24 @@ import assert from 'assert'; // a loader that asserts that the defaultResolve will throw "not found" // (skipping the top-level main of course) let mainLoad = true; -export async function resolve (specifier, base, defaultResolve) { - if (mainLoad) { - mainLoad = false; - return defaultResolve(specifier, base); - } - try { - await defaultResolve(specifier, base); - } - catch (e) { - assert.strictEqual(e.code, 'MODULE_NOT_FOUND'); - return { - format: 'builtin', - url: 'fs' - }; - } - assert.fail(`Module resolution for ${specifier} should be throw MODULE_NOT_FOUND`); +export default ({ resolve: parentResolve }) => { + return { + async resolve (specifier, base) { + if (mainLoad) { + mainLoad = false; + return parentResolve(specifier, base); + } + try { + await parentResolve(specifier, base); + } + catch (e) { + assert.strictEqual(e.code, 'MODULE_NOT_FOUND'); + return { + format: 'builtin', + url: 'fs' + }; + } + assert.fail(`Module resolution for ${specifier} should be throw MODULE_NOT_FOUND`); + } + }; } From 0d5badfa651c9a392e5297c090592264016d0354 Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Wed, 21 Feb 2018 15:35:14 -0600 Subject: [PATCH 2/9] default dynamicInstantiate to throw and add type check --- lib/internal/errors.js | 5 ++ lib/internal/process/modules.js | 87 ++++++++++++++-------- test/es-module/test-esm-loader-multiple.js | 4 +- 3 files changed, 62 insertions(+), 34 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index a4a79d671e4938..639f63d3b5460f 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -800,8 +800,13 @@ E('ERR_IPC_CHANNEL_CLOSED', 'Channel closed', Error); E('ERR_IPC_DISCONNECTED', 'IPC channel is already disconnected', Error); E('ERR_IPC_ONE_PIPE', 'Child process can have only one IPC pipe', Error); E('ERR_IPC_SYNC_FORK', 'IPC cannot be used with synchronous forks', Error); +E('ERR_LOADER_HOOK_BAD_TYPE', + 'ES Module loader hook %s must be of type %s', TypeError); E('ERR_METHOD_NOT_IMPLEMENTED', 'The %s method is not implemented', Error); E('ERR_MISSING_ARGS', missingArgs, TypeError); +E('ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK', + 'The ES Module loader may not return a format of \'dynamic\' when no ' + + 'dynamicInstantiate function was provided', TypeError); E('ERR_MISSING_MODULE', 'Cannot find module %s', Error); E('ERR_MODULE_RESOLUTION_LEGACY', '%s not found by import in %s.' + diff --git a/lib/internal/process/modules.js b/lib/internal/process/modules.js index 09722b80c19afb..3d3ef9ad683991 100644 --- a/lib/internal/process/modules.js +++ b/lib/internal/process/modules.js @@ -2,14 +2,28 @@ const { setImportModuleDynamicallyCallback, - setInitializeImportMetaObjectCallback + setInitializeImportMetaObjectCallback, } = internalBinding('module_wrap'); +const getOwnPropertyDescriptor = Object.getOwnPropertyDescriptor; +const hasOwnProperty = Function.call.bind(Object.prototype.hasOwnProperty); +const apply = Reflect.apply; + +const errors = require('internal/errors'); const { getURLFromFilePath } = require('internal/url'); const Loader = require('internal/loader/Loader'); const path = require('path'); const { URL } = require('url'); +// fires a getter or reads the value off a descriptor +function grabPropertyOffDescriptor(object, descriptor) { + if (hasOwnProperty(descriptor, 'value')) { + return descriptor.value; + } else { + return apply(descriptor.get, object, []); + } +} + function normalizeReferrerURL(referrer) { if (typeof referrer === 'string' && path.isAbsolute(referrer)) { return getURLFromFilePath(referrer).href; @@ -35,45 +49,54 @@ exports.setup = function() { const loaderPromise = (async () => { const { userLoaders } = process.binding('config'); if (userLoaders) { - let previous = { - resolve: (url, referrer) => { - return require('internal/loader/DefaultResolve')(url, referrer); - }, - dynamicInstantiate: null + let resolve = (url, referrer) => { + return require('internal/loader/DefaultResolve')(url, referrer); + }; + let dynamicInstantiate = (url) => { + throw new errors.Error('ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK') }; for (var i = 0; i < userLoaders.length; i++) { const loaderSpecifier = userLoaders[i]; const { default: factory } = await ESMLoader.import(loaderSpecifier); - const parent = previous; - previous = factory({ + let cachedResolve = resolve; + let cachedDynamicInstantiate = dynamicInstantiate; + let next = factory({ __proto__: null, - resolve: parent.resolve ? Object.setPrototypeOf( - async (url, referrer) => { - const ret = await parent.resolve(url, referrer); - return { - __proto__: null, - url: `${ret.url}`, - format: `${ret.format}`, - }; - }, - null - ) : null, - dynamicInstantiate: parent.dynamicInstantiate ? Object.setPrototypeOf( - async (url) => { - const ret = await parent.dynamicInstantiate(url); - return { - __proto__: null, - exports: ret.exports, - execute: ret.execute - }; - }, - null - ) : null + resolve: Object.setPrototypeOf(async (url, referrer) => { + const ret = await cachedResolve(url, referrer); + return { + __proto__: null, + url: `${ret.url}`, + format: `${ret.format}`, + }; + }, null), + dynamicInstantiate: Object.setPrototypeOf(async url => { + const ret = await dynamicInstantiate(url); + return { + __proto__: null, + exports: ret.exports, + execute: ret.execute, + }; + }, null), }); + const resolveDesc = getOwnPropertyDescriptor(next, 'resolve'); + if (resolveDesc !== undefined) { + resolve = grabPropertyOffDescriptor(next, resolveDesc); + if (typeof resolve !== 'function') { + throw new errors.TypeError('ERR_LOADER_HOOK_BAD_TYPE', 'resolve', 'function'); + } + } + const dynamicInstantiateDesc = getOwnPropertyDescriptor(next, 'dynamicInstantiate'); + if (dynamicInstantiateDesc !== undefined) { + dynamicInstantiate = grabPropertyOffDescriptor(next, dynamicInstantiateDesc); + if (typeof dynamicInstantiate !== 'function') { + throw new errors.TypeError('ERR_LOADER_HOOK_BAD_TYPE', 'dynamicInstantiate', 'function'); + } + } } ESMLoader.hook({ - resolve: previous.resolve !== undefined ? previous.resolve.bind(previous) : undefined, - dynamicInstantiate: previous.dynamicInstantiate !== undefined ? previous.dynamicInstantiate.bind(previous) : undefined + resolve, + dynamicInstantiate }); } return ESMLoader; diff --git a/test/es-module/test-esm-loader-multiple.js b/test/es-module/test-esm-loader-multiple.js index d38acfdb48b090..93d8179ffd8dfb 100644 --- a/test/es-module/test-esm-loader-multiple.js +++ b/test/es-module/test-esm-loader-multiple.js @@ -24,7 +24,7 @@ const dependency = path.join(__dirname, 'esm-export-url.mjs'); logger, ], { - stdio: 'pipe', + stdio: 'pipe' } ); assert.strictEqual(result.status, 0); @@ -46,7 +46,7 @@ const dependency = path.join(__dirname, 'esm-export-url.mjs'); logger, ], { - stdio: 'pipe', + stdio: 'pipe' } ); assert.strictEqual(result.status, 0); From 64fa4d8192a09214a74f7326cc33dd2b1ef4dc83 Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Wed, 21 Feb 2018 15:37:08 -0600 Subject: [PATCH 3/9] doc touchup --- doc/api/esm.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 431569dbcc3409..8f57a882a40964 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -109,8 +109,7 @@ To customize the default module resolution, loader hooks can optionally be provided via a `--loader ./loader-name.mjs` argument to Node. This argument can be passed multiple times to compose loaders like `--loader ./loader-coverage.mjs --loader ./loader-mocking.mjs`. The last loader -will be used for module loading and must explicitly call to the parent loader -in order to provide compose behavior. +must explicitly call to the parent loader in order to provide compose behavior. When hooks are used they only apply to ES module loading and not to any CommonJS modules loaded. @@ -118,8 +117,8 @@ CommonJS modules loaded. All loaders are created by invoking the default export of their module as a function. The parameters given to the the function is a single object with properties to call the `resolve` and `dynamicInstantiate` hooks of the parent -loader. The default loader has a `resolve` hook and `null` for the value -`dynamicInstantiate`. +loader. The default loader has a `resolve` hook and a function that throws for +the value of `dynamicInstantiate`. ### Resolve hook From e88059f2242acbd129e099aabaf88cc42d391929 Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Wed, 21 Feb 2018 15:40:56 -0600 Subject: [PATCH 4/9] type check test --- ...oader-invalid-dynamic-instantiate-type.mjs | 8 ++++ .../es-module/loader-invalid-resolve-type.mjs | 8 ++++ test/es-module/test-esm-loader-types.mjs | 45 +++++++++++++++++++ 3 files changed, 61 insertions(+) create mode 100644 test/es-module/loader-invalid-dynamic-instantiate-type.mjs create mode 100644 test/es-module/loader-invalid-resolve-type.mjs create mode 100644 test/es-module/test-esm-loader-types.mjs diff --git a/test/es-module/loader-invalid-dynamic-instantiate-type.mjs b/test/es-module/loader-invalid-dynamic-instantiate-type.mjs new file mode 100644 index 00000000000000..80877ff9082e4f --- /dev/null +++ b/test/es-module/loader-invalid-dynamic-instantiate-type.mjs @@ -0,0 +1,8 @@ +/* eslint-disable node-core/required-modules */ +import url from 'url'; + +export default ({ + resolve: parentResolve +}) => ({ + dynamicInstantiate: 'yes' +}); diff --git a/test/es-module/loader-invalid-resolve-type.mjs b/test/es-module/loader-invalid-resolve-type.mjs new file mode 100644 index 00000000000000..ca7af813e2bbe7 --- /dev/null +++ b/test/es-module/loader-invalid-resolve-type.mjs @@ -0,0 +1,8 @@ +/* eslint-disable node-core/required-modules */ +import url from 'url'; + +export default ({ + resolve: parentResolve +}) => ({ + resolve: 'yes' +}); diff --git a/test/es-module/test-esm-loader-types.mjs b/test/es-module/test-esm-loader-types.mjs new file mode 100644 index 00000000000000..ed43411bd59da3 --- /dev/null +++ b/test/es-module/test-esm-loader-types.mjs @@ -0,0 +1,45 @@ +// Flags: --experimental-modules +'use strict'; + +require('../common'); +const { spawnSync } = require('child_process'); +const assert = require('assert'); +const path = require('path'); +const url = require('url'); + +const invalid_resolve = path.join(__dirname, 'loader-invalid-resolve-type.mjs'); +const invalid_dynamic_instantiate = path.join(__dirname, 'loader-invalid-dynamic-instantiate-type.mjs'); + +{ + const result = spawnSync( + process.execPath, + [ + '--experimental-modules', + '--loader', + invalid_resolve, + __filename + ], + { + stdio: 'pipe' + } + ); + assert.strictEqual(result.status, 1); + assert(/ERR_LOADER_HOOK_BAD_TYPE/.test(result.stderr)); +} + +{ + const result = spawnSync( + process.execPath, + [ + '--experimental-modules', + '--loader', + invalid_dynamic_instantiate, + __filename + ], + { + stdio: 'pipe' + } + ); + assert.strictEqual(result.status, 1); + assert(/ERR_LOADER_HOOK_BAD_TYPE/.test(result.stderr)); +} From b03d27d915f756b6de347c84f41680c0135eeb89 Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Thu, 22 Feb 2018 08:07:38 -0600 Subject: [PATCH 5/9] doc nits --- doc/api/esm.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 8f57a882a40964..456a7729de5695 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -106,7 +106,7 @@ fs.readFile('./foo.txt', (err, body) => { To customize the default module resolution, loader hooks can optionally be -provided via a `--loader ./loader-name.mjs` argument to Node. This argument +provided via a `--loader ./loader-name.mjs` argument to Node.js. This argument can be passed multiple times to compose loaders like `--loader ./loader-coverage.mjs --loader ./loader-mocking.mjs`. The last loader must explicitly call to the parent loader in order to provide compose behavior. @@ -115,7 +115,7 @@ When hooks are used they only apply to ES module loading and not to any CommonJS modules loaded. All loaders are created by invoking the default export of their module as a -function. The parameters given to the the function is a single object with +function. The parameters given to the function are a single object with properties to call the `resolve` and `dynamicInstantiate` hooks of the parent loader. The default loader has a `resolve` hook and a function that throws for the value of `dynamicInstantiate`. From 3bd6e7c001a7668536092b2f1af35b1dc2c376f3 Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Thu, 22 Feb 2018 08:34:44 -0600 Subject: [PATCH 6/9] nits and lints --- doc/api/errors.md | 6 +++ doc/api/esm.md | 9 ++-- lib/internal/process/modules.js | 52 +++++++++++-------- src/node_config.cc | 10 ++-- ...oader-invalid-dynamic-instantiate-type.mjs | 1 - .../es-module/loader-invalid-resolve-type.mjs | 1 - test/es-module/test-esm-loader-types.mjs | 45 ---------------- 7 files changed, 45 insertions(+), 79 deletions(-) delete mode 100644 test/es-module/test-esm-loader-types.mjs diff --git a/doc/api/errors.md b/doc/api/errors.md index b9c52ad0d9af89..f2c85357823701 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1229,6 +1229,11 @@ An attempt was made to open an IPC communication channel with a synchronously forked Node.js process. See the documentation for the [`child_process`][] module for more information. + +### ERR_LOADER_HOOK_BAD_TYPE + +An Loader defined an invalid value for a hook. See the documentation for [ECMAScript Modules][] for more information. + ### ERR_METHOD_NOT_IMPLEMENTED @@ -1630,6 +1635,7 @@ Creation of a [`zlib`][] object failed due to incorrect configuration. [`cipher.getAuthTag()`]: crypto.html#crypto_cipher_getauthtag [`crypto.timingSafeEqual()`]: crypto.html#crypto_crypto_timingsafeequal_a_b [`dgram.createSocket()`]: dgram.html#dgram_dgram_createsocket_options_callback +[ECMAScript Modules]: esm.html [`ERR_INVALID_ARG_TYPE`]: #ERR_INVALID_ARG_TYPE [`EventEmitter`]: events.html#events_class_eventemitter [`fs.symlink()`]: fs.html#fs_fs_symlink_target_path_type_callback diff --git a/doc/api/esm.md b/doc/api/esm.md index 456a7729de5695..0deb68ef47a81f 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -129,7 +129,7 @@ given module specifier and parent file URL: const baseURL = new URL('file://'); baseURL.pathname = `${process.cwd()}/`; -export default function (parent) { +export default function(parent) { return { async resolve(specifier, parentModuleURL = baseURL) { @@ -176,7 +176,7 @@ const JS_EXTENSIONS = new Set(['.js', '.mjs']); const baseURL = new URL('file://'); baseURL.pathname = `${process.cwd()}/`; -export default function (parent) { +export default function(parent) { return { resolve(specifier, parentModuleURL = baseURL) { if (builtins.includes(specifier)) { @@ -185,7 +185,8 @@ export default function (parent) { format: 'builtin' }; } - if (/^\.{0,2}[/]/.test(specifier) !== true && !specifier.startsWith('file:')) { + if (/^\.{0,2}[/]/.test(specifier) !== true && + !specifier.startsWith('file:')) { // For node_modules support: // return parent.resolve(specifier, parentModuleURL); throw new Error( @@ -223,7 +224,7 @@ This hook is called only for modules that return `format: "dynamic"` from the `resolve` hook. ```js -export default function (parent) { +export default function(parent) { return { dynamicInstantiate(url) { return { diff --git a/lib/internal/process/modules.js b/lib/internal/process/modules.js index 3d3ef9ad683991..6cba398b159ed8 100644 --- a/lib/internal/process/modules.js +++ b/lib/internal/process/modules.js @@ -53,44 +53,50 @@ exports.setup = function() { return require('internal/loader/DefaultResolve')(url, referrer); }; let dynamicInstantiate = (url) => { - throw new errors.Error('ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK') + throw new errors.Error('ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK'); }; for (var i = 0; i < userLoaders.length; i++) { const loaderSpecifier = userLoaders[i]; const { default: factory } = await ESMLoader.import(loaderSpecifier); - let cachedResolve = resolve; - let cachedDynamicInstantiate = dynamicInstantiate; - let next = factory({ + const cachedResolve = resolve; + const cachedDynamicInstantiate = dynamicInstantiate; + const next = factory({ __proto__: null, resolve: Object.setPrototypeOf(async (url, referrer) => { - const ret = await cachedResolve(url, referrer); - return { - __proto__: null, - url: `${ret.url}`, - format: `${ret.format}`, - }; - }, null), - dynamicInstantiate: Object.setPrototypeOf(async url => { - const ret = await dynamicInstantiate(url); - return { - __proto__: null, - exports: ret.exports, - execute: ret.execute, - }; - }, null), + const ret = await cachedResolve(url, referrer); + return { + __proto__: null, + url: `${ret.url}`, + format: `${ret.format}`, + }; + }, null), + dynamicInstantiate: Object.setPrototypeOf(async (url) => { + const ret = await cachedDynamicInstantiate(url); + return { + __proto__: null, + exports: ret.exports, + execute: ret.execute, + }; + }, null), }); const resolveDesc = getOwnPropertyDescriptor(next, 'resolve'); if (resolveDesc !== undefined) { resolve = grabPropertyOffDescriptor(next, resolveDesc); if (typeof resolve !== 'function') { - throw new errors.TypeError('ERR_LOADER_HOOK_BAD_TYPE', 'resolve', 'function'); + throw new errors.TypeError('ERR_LOADER_HOOK_BAD_TYPE', + 'resolve', 'function'); } } - const dynamicInstantiateDesc = getOwnPropertyDescriptor(next, 'dynamicInstantiate'); + const dynamicInstantiateDesc = getOwnPropertyDescriptor( + next, + 'dynamicInstantiate'); if (dynamicInstantiateDesc !== undefined) { - dynamicInstantiate = grabPropertyOffDescriptor(next, dynamicInstantiateDesc); + dynamicInstantiate = grabPropertyOffDescriptor( + next, + dynamicInstantiateDesc); if (typeof dynamicInstantiate !== 'function') { - throw new errors.TypeError('ERR_LOADER_HOOK_BAD_TYPE', 'dynamicInstantiate', 'function'); + throw new errors.TypeError('ERR_LOADER_HOOK_BAD_TYPE', + 'dynamicInstantiate', 'function'); } } } diff --git a/src/node_config.cc b/src/node_config.cc index 80ffb7b78425ca..5bbd01ac0792c5 100644 --- a/src/node_config.cc +++ b/src/node_config.cc @@ -75,14 +75,14 @@ static void InitConfig(Local target, auto loaders = v8::Array::New(isolate, config_userland_loaders.size()); for (size_t i = 0; i < config_userland_loaders.size(); i++) { auto item = config_userland_loaders[i]; - loaders->Set( - i, - String::NewFromUtf8(isolate, + auto user_loader = String::NewFromUtf8(isolate, item.data(), - v8::NewStringType::kNormal).ToLocalChecked()); + v8::NewStringType::kNormal).ToLocalChecked(); + loaders->Set(context, i, user_loader).FromJust(); } - loaders->SetIntegrityLevel(context, v8::IntegrityLevel::kFrozen); + loaders->SetIntegrityLevel(context, v8::IntegrityLevel::kFrozen) + .FromJust(); target->DefineOwnProperty( context, FIXED_ONE_BYTE_STRING(isolate, "userLoaders"), diff --git a/test/es-module/loader-invalid-dynamic-instantiate-type.mjs b/test/es-module/loader-invalid-dynamic-instantiate-type.mjs index 80877ff9082e4f..b20ac602b85b9a 100644 --- a/test/es-module/loader-invalid-dynamic-instantiate-type.mjs +++ b/test/es-module/loader-invalid-dynamic-instantiate-type.mjs @@ -1,5 +1,4 @@ /* eslint-disable node-core/required-modules */ -import url from 'url'; export default ({ resolve: parentResolve diff --git a/test/es-module/loader-invalid-resolve-type.mjs b/test/es-module/loader-invalid-resolve-type.mjs index ca7af813e2bbe7..fc9a78ec1d85dc 100644 --- a/test/es-module/loader-invalid-resolve-type.mjs +++ b/test/es-module/loader-invalid-resolve-type.mjs @@ -1,5 +1,4 @@ /* eslint-disable node-core/required-modules */ -import url from 'url'; export default ({ resolve: parentResolve diff --git a/test/es-module/test-esm-loader-types.mjs b/test/es-module/test-esm-loader-types.mjs deleted file mode 100644 index ed43411bd59da3..00000000000000 --- a/test/es-module/test-esm-loader-types.mjs +++ /dev/null @@ -1,45 +0,0 @@ -// Flags: --experimental-modules -'use strict'; - -require('../common'); -const { spawnSync } = require('child_process'); -const assert = require('assert'); -const path = require('path'); -const url = require('url'); - -const invalid_resolve = path.join(__dirname, 'loader-invalid-resolve-type.mjs'); -const invalid_dynamic_instantiate = path.join(__dirname, 'loader-invalid-dynamic-instantiate-type.mjs'); - -{ - const result = spawnSync( - process.execPath, - [ - '--experimental-modules', - '--loader', - invalid_resolve, - __filename - ], - { - stdio: 'pipe' - } - ); - assert.strictEqual(result.status, 1); - assert(/ERR_LOADER_HOOK_BAD_TYPE/.test(result.stderr)); -} - -{ - const result = spawnSync( - process.execPath, - [ - '--experimental-modules', - '--loader', - invalid_dynamic_instantiate, - __filename - ], - { - stdio: 'pipe' - } - ); - assert.strictEqual(result.status, 1); - assert(/ERR_LOADER_HOOK_BAD_TYPE/.test(result.stderr)); -} From e2a89b9c6f50ce5745b952243ca252b39114bb27 Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Fri, 23 Feb 2018 09:52:15 -0600 Subject: [PATCH 7/9] address guybedford feedback --- doc/api/esm.md | 44 ++++++++++++------ lib/internal/process/modules.js | 18 +++++--- test/es-module/test-esm-loader-types.js | 46 +++++++++++++++++++ .../es-module-loaders/loader-shared-dep.mjs | 2 +- 4 files changed, 90 insertions(+), 20 deletions(-) create mode 100644 test/es-module/test-esm-loader-types.js diff --git a/doc/api/esm.md b/doc/api/esm.md index 0deb68ef47a81f..4bcc185925c631 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -126,6 +126,8 @@ The resolve hook returns the resolved file URL and module format for a given module specifier and parent file URL: ```js +// example loader that treats all files within the current working directory as +// ECMAScript Modules const baseURL = new URL('file://'); baseURL.pathname = `${process.cwd()}/`; @@ -133,10 +135,15 @@ export default function(parent) { return { async resolve(specifier, parentModuleURL = baseURL) { - return { - url: new URL(specifier, parentModuleURL).href, - format: 'esm' - }; + const location = new URL(specifier, parentModuleURL); + if (locations.host === baseURL.host && + location.pathname.startsWith(baseURL.pathname)) { + return { + url: location.href, + format: 'esm' + }; + } + return parent.resolve(specifier, parentModuleURL); } }; } @@ -190,7 +197,9 @@ export default function(parent) { // For node_modules support: // return parent.resolve(specifier, parentModuleURL); throw new Error( - `imports must begin with '/', './', or '../'; '${specifier}' does not`); + `imports must begin with '/', './', or '../'; '${ + specifier + }' does not`); } const resolved = new URL(specifier, parentModuleURL); const ext = path.extname(resolved.pathname); @@ -224,16 +233,25 @@ This hook is called only for modules that return `format: "dynamic"` from the `resolve` hook. ```js +// example loader that can generate modules for .txt files +// that resolved to a 'dynamic' format +import fs from 'fs'; +import util from 'util'; export default function(parent) { return { - dynamicInstantiate(url) { - return { - exports: ['customExportName'], - execute: (exports) => { - // get and set functions provided for pre-allocated export names - exports.customExportName.set('value'); - } - }; + async dynamicInstantiate(url) { + const location = new URL(url); + if (location.pathname.slice(-4) === '.txt') { + const text = String(await util.promisify(fs.readFile)(location)); + return { + exports: ['text'], + execute: (exports) => { + // get and set functions provided for pre-allocated export names + exports.text.set(text); + } + }; + } + return parent.dynamicInstantiate(url); } }; } diff --git a/lib/internal/process/modules.js b/lib/internal/process/modules.js index 6cba398b159ed8..4dd61099bfeeb6 100644 --- a/lib/internal/process/modules.js +++ b/lib/internal/process/modules.js @@ -37,7 +37,9 @@ function initializeImportMetaObject(wrap, meta) { let loaderResolve; exports.loaderPromise = new Promise((resolve, reject) => { - loaderResolve = resolve; + loaderResolve = v => { + resolve(v); + } }); exports.ESMLoader = undefined; @@ -45,10 +47,12 @@ exports.ESMLoader = undefined; exports.setup = function() { setInitializeImportMetaObjectCallback(initializeImportMetaObject); - const ESMLoader = new Loader(); + const RuntimeLoader = new Loader(); const loaderPromise = (async () => { const { userLoaders } = process.binding('config'); if (userLoaders) { + const BootstrapLoader = new Loader(); + exports.ESMLoader = BootstrapLoader; let resolve = (url, referrer) => { return require('internal/loader/DefaultResolve')(url, referrer); }; @@ -57,7 +61,8 @@ exports.setup = function() { }; for (var i = 0; i < userLoaders.length; i++) { const loaderSpecifier = userLoaders[i]; - const { default: factory } = await ESMLoader.import(loaderSpecifier); + const { default: factory } = await BootstrapLoader.import( + loaderSpecifier); const cachedResolve = resolve; const cachedDynamicInstantiate = dynamicInstantiate; const next = factory({ @@ -100,12 +105,13 @@ exports.setup = function() { } } } - ESMLoader.hook({ + RuntimeLoader.hook({ resolve, dynamicInstantiate }); } - return ESMLoader; + exports.ESMLoader = RuntimeLoader; + return RuntimeLoader; })(); loaderResolve(loaderPromise); @@ -114,5 +120,5 @@ exports.setup = function() { return loader.import(specifier, normalizeReferrerURL(referrer)); }); - exports.ESMLoader = ESMLoader; + exports.RuntimeLoader = RuntimeLoader; }; diff --git a/test/es-module/test-esm-loader-types.js b/test/es-module/test-esm-loader-types.js new file mode 100644 index 00000000000000..46bef2d6e6206e --- /dev/null +++ b/test/es-module/test-esm-loader-types.js @@ -0,0 +1,46 @@ +// Flags: --experimental-modules +'use strict'; + +require('../common'); +const { spawnSync } = require('child_process'); +const assert = require('assert'); +const path = require('path'); + +const invalid_resolve = path.join(__dirname, 'loader-invalid-resolve-type.mjs'); +const invalid_dynamic_instantiate = path.join( + __dirname, + 'loader-invalid-dynamic-instantiate-type.mjs'); + +{ + const result = spawnSync( + process.execPath, + [ + '--experimental-modules', + '--loader', + invalid_resolve, + __filename + ], + { + stdio: 'pipe' + } + ); + assert.strictEqual(result.status, 1); + assert(/ERR_LOADER_HOOK_BAD_TYPE/.test(result.stderr)); +} + +{ + const result = spawnSync( + process.execPath, + [ + '--experimental-modules', + '--loader', + invalid_dynamic_instantiate, + __filename + ], + { + stdio: 'pipe' + } + ); + assert.strictEqual(result.status, 1); + assert(/ERR_LOADER_HOOK_BAD_TYPE/.test(result.stderr)); +} diff --git a/test/fixtures/es-module-loaders/loader-shared-dep.mjs b/test/fixtures/es-module-loaders/loader-shared-dep.mjs index 4410363609a494..745ca1712b86ee 100644 --- a/test/fixtures/es-module-loaders/loader-shared-dep.mjs +++ b/test/fixtures/es-module-loaders/loader-shared-dep.mjs @@ -3,7 +3,7 @@ import assert from 'assert'; export default ({ resolve: parentResolve }) => { return { - resolve(specifier, base, defaultResolve) { + resolve(specifier, base) { assert.strictEqual(dep.format, 'esm'); return parentResolve(specifier, base); } From 4f974f0ea1a746c7b9a794e5aed8d64b23be3f4b Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Tue, 27 Feb 2018 11:12:09 -0600 Subject: [PATCH 8/9] doc style --- doc/api/errors.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index f2c85357823701..fe049ba9c7cd36 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1232,7 +1232,8 @@ for more information. ### ERR_LOADER_HOOK_BAD_TYPE -An Loader defined an invalid value for a hook. See the documentation for [ECMAScript Modules][] for more information. +A Loader defined an invalid value for a hook. See the documentation for +[ECMAScript Modules][] for more information. ### ERR_METHOD_NOT_IMPLEMENTED From 49068605d354f8058697b1598baf2fe3c52272f5 Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Mon, 5 Mar 2018 09:24:49 -0600 Subject: [PATCH 9/9] fixup for lint --- doc/api/errors.md | 7 +++++++ lib/internal/process/modules.js | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index fe049ba9c7cd36..5f0d1648614b3a 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1249,6 +1249,13 @@ 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 + +A Loader resolved an import to a "dynamic" format, but no hook for +`dynamicInstantiate` was declared. See Loader hook documentation in +[ECMAScript Modules][]. + ### ERR_MISSING_MODULE diff --git a/lib/internal/process/modules.js b/lib/internal/process/modules.js index 4dd61099bfeeb6..94baa7825eee9c 100644 --- a/lib/internal/process/modules.js +++ b/lib/internal/process/modules.js @@ -37,9 +37,9 @@ function initializeImportMetaObject(wrap, meta) { let loaderResolve; exports.loaderPromise = new Promise((resolve, reject) => { - loaderResolve = v => { + loaderResolve = (v) => { resolve(v); - } + }; }); exports.ESMLoader = undefined;