From 7a01402b4e6ee68d39aad147cbb761a4172db699 Mon Sep 17 00:00:00 2001 From: guybedford Date: Sun, 22 Oct 2017 14:42:50 +0200 Subject: [PATCH] module: fix hook module CJS dependency loading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It can be useful to load dependencies as part of the loader hook definition file. This fixes a bug where `import x from 'x'` would always return `x` as `undefined` if the import was made in a loader hooks definition module. A parallel change to the CJS loading injection process meant that the CJS module wasn't being injected into the correct loader instance, which is corrected here with a test. PR-URL: https://github.com/nodejs/node/pull/16381 Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Michaël Zasso --- lib/module.js | 3 ++- test/es-module/test-esm-loader-dependency.mjs | 5 +++++ test/fixtures/es-module-loaders/loader-dep.js | 1 + test/fixtures/es-module-loaders/loader-with-dep.mjs | 7 +++++++ 4 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 test/es-module/test-esm-loader-dependency.mjs create mode 100644 test/fixtures/es-module-loaders/loader-dep.js create mode 100644 test/fixtures/es-module-loaders/loader-with-dep.mjs diff --git a/lib/module.js b/lib/module.js index d625bf96b932b5..2fba9d7bfac517 100644 --- a/lib/module.js +++ b/lib/module.js @@ -447,7 +447,8 @@ Module._load = function(request, parent, isMain) { ESMLoader = new Loader(); const userLoader = process.binding('config').userLoader; if (userLoader) { - const hooks = await new Loader().import(userLoader); + const hooks = await ESMLoader.import(userLoader); + ESMLoader = new Loader(); ESMLoader.hook(hooks); } } diff --git a/test/es-module/test-esm-loader-dependency.mjs b/test/es-module/test-esm-loader-dependency.mjs new file mode 100644 index 00000000000000..5d05118dbf2879 --- /dev/null +++ b/test/es-module/test-esm-loader-dependency.mjs @@ -0,0 +1,5 @@ +// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-with-dep.mjs +/* eslint-disable required-modules */ +import './test-esm-ok.mjs'; + +// We just test that this module doesn't fail loading diff --git a/test/fixtures/es-module-loaders/loader-dep.js b/test/fixtures/es-module-loaders/loader-dep.js new file mode 100644 index 00000000000000..cf821afec16e2c --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-dep.js @@ -0,0 +1 @@ +exports.format = 'esm'; diff --git a/test/fixtures/es-module-loaders/loader-with-dep.mjs b/test/fixtures/es-module-loaders/loader-with-dep.mjs new file mode 100644 index 00000000000000..944e6e438c5cbd --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-with-dep.mjs @@ -0,0 +1,7 @@ +import dep from './loader-dep.js'; +export function resolve (specifier, base, defaultResolve) { + return { + url: defaultResolve(specifier, base).url, + format: dep.format + }; +}