From d7192c4e6ad6ff01c946b8783295234c3acb9156 Mon Sep 17 00:00:00 2001 From: Jan Krems Date: Sun, 1 Oct 2017 10:31:04 -0700 Subject: [PATCH] module: Set dynamic import callback This is an initial implementation to support dynamic import in both scripts and modules. It's off by default since support for dynamic import is still flagged in V8. Without setting the V8 flag, this code won't be executed. This initial version does not support importing into vm contexts. Backport-PR-URL: https://github.com/nodejs/node/pull/17823 PR-URL: https://github.com/nodejs/node/pull/15713 Reviewed-By: Timothy Gu Reviewed-By: Bradley Farias --- .eslintignore | 1 + lib/internal/loader/Loader.js | 21 +++- lib/internal/loader/ModuleWrap.js | 8 +- lib/module.js | 1 + src/env.h | 1 + src/module_wrap.cc | 59 +++++++++++ src/module_wrap.h | 2 + test/es-module/test-esm-dynamic-import.js | 113 ++++++++++++++++++++++ 8 files changed, 202 insertions(+), 4 deletions(-) create mode 100644 test/es-module/test-esm-dynamic-import.js diff --git a/.eslintignore b/.eslintignore index 4b6e6b5e0fa94a..36833076ed8708 100644 --- a/.eslintignore +++ b/.eslintignore @@ -1,6 +1,7 @@ lib/internal/v8_prof_polyfill.js lib/punycode.js test/addons/??_* +test/es-module/test-esm-dynamic-import.js test/fixtures tools/eslint tools/icu diff --git a/lib/internal/loader/Loader.js b/lib/internal/loader/Loader.js index 49c8699771e819..b8fe9ccfc614b8 100644 --- a/lib/internal/loader/Loader.js +++ b/lib/internal/loader/Loader.js @@ -1,8 +1,12 @@ 'use strict'; -const { getURLFromFilePath } = require('internal/url'); +const path = require('path'); +const { getURLFromFilePath, URL } = require('internal/url'); -const { createDynamicModule } = require('internal/loader/ModuleWrap'); +const { + createDynamicModule, + setImportModuleDynamicallyCallback +} = require('internal/loader/ModuleWrap'); const ModuleMap = require('internal/loader/ModuleMap'); const ModuleJob = require('internal/loader/ModuleJob'); @@ -24,6 +28,13 @@ function getURLStringForCwd() { } } +function normalizeReferrerURL(referrer) { + if (typeof referrer === 'string' && path.isAbsolute(referrer)) { + return getURLFromFilePath(referrer).href; + } + return new URL(referrer).href; +} + /* 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. */ @@ -129,6 +140,12 @@ class Loader { const module = await job.run(); return module.namespace(); } + + static registerImportDynamicallyCallback(loader) { + setImportModuleDynamicallyCallback(async (referrer, specifier) => { + return loader.import(specifier, normalizeReferrerURL(referrer)); + }); + } } Loader.validFormats = ['esm', 'cjs', 'builtin', 'addon', 'json', 'dynamic']; Object.setPrototypeOf(Loader.prototype, null); diff --git a/lib/internal/loader/ModuleWrap.js b/lib/internal/loader/ModuleWrap.js index 0ee05ca81ffbb9..63ce6fd70b5bbe 100644 --- a/lib/internal/loader/ModuleWrap.js +++ b/lib/internal/loader/ModuleWrap.js @@ -1,7 +1,10 @@ 'use strict'; -const { ModuleWrap } = - require('internal/process').internalBinding('module_wrap'); +const { + ModuleWrap, + setImportModuleDynamicallyCallback +} = require('internal/process').internalBinding('module_wrap'); + const debug = require('util').debuglog('esm'); const ArrayJoin = Function.call.bind(Array.prototype.join); const ArrayMap = Function.call.bind(Array.prototype.map); @@ -60,5 +63,6 @@ const createDynamicModule = (exports, url = '', evaluate) => { module.exports = { createDynamicModule, + setImportModuleDynamicallyCallback, ModuleWrap }; diff --git a/lib/module.js b/lib/module.js index ed9d1a78d476a1..642e5cb1f1532d 100644 --- a/lib/module.js +++ b/lib/module.js @@ -462,6 +462,7 @@ Module._load = function(request, parent, isMain) { ESMLoader.hook(hooks); } } + Loader.registerImportDynamicallyCallback(ESMLoader); await ESMLoader.import(getURLFromFilePath(request).pathname); })() .catch((e) => { diff --git a/src/env.h b/src/env.h index 9568b1545ba1d3..15eacdb54fa139 100644 --- a/src/env.h +++ b/src/env.h @@ -316,6 +316,7 @@ class ModuleWrap; V(context, v8::Context) \ V(domain_array, v8::Array) \ V(domains_stack_array, v8::Array) \ + V(host_import_module_dynamically_callback, v8::Function) \ V(http2ping_constructor_template, v8::ObjectTemplate) \ V(http2stream_constructor_template, v8::ObjectTemplate) \ V(http2settings_constructor_template, v8::ObjectTemplate) \ diff --git a/src/module_wrap.cc b/src/module_wrap.cc index dfba4d5b300f66..4368fc6d0586c3 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -550,6 +550,62 @@ void ModuleWrap::Resolve(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(result.FromJust().ToObject(env)); } +static MaybeLocal ImportModuleDynamically( + Local context, + Local referrer, + Local specifier) { + Isolate* iso = context->GetIsolate(); + Environment* env = Environment::GetCurrent(context); + v8::EscapableHandleScope handle_scope(iso); + + if (env->context() != context) { + auto maybe_resolver = Promise::Resolver::New(context); + Local resolver; + if (maybe_resolver.ToLocal(&resolver)) { + // TODO(jkrems): Turn into proper error object w/ code + Local error = v8::Exception::Error( + OneByteString(iso, "import() called outside of main context")); + if (resolver->Reject(context, error).IsJust()) { + return handle_scope.Escape(resolver.As()); + } + } + return MaybeLocal(); + } + + Local import_callback = + env->host_import_module_dynamically_callback(); + Local import_args[] = { + referrer->GetResourceName(), + Local(specifier) + }; + MaybeLocal maybe_result = import_callback->Call(context, + v8::Undefined(iso), + 2, + import_args); + + Local result; + if (maybe_result.ToLocal(&result)) { + return handle_scope.Escape(result.As()); + } + return MaybeLocal(); +} + +void ModuleWrap::SetImportModuleDynamicallyCallback( + const FunctionCallbackInfo& args) { + Isolate* iso = args.GetIsolate(); + Environment* env = Environment::GetCurrent(args); + HandleScope handle_scope(iso); + if (!args[0]->IsFunction()) { + env->ThrowError("first argument is not a function"); + return; + } + + Local import_callback = args[0].As(); + env->set_host_import_module_dynamically_callback(import_callback); + + iso->SetHostImportModuleDynamicallyCallback(ImportModuleDynamically); +} + void ModuleWrap::Initialize(Local target, Local unused, Local context) { @@ -567,6 +623,9 @@ void ModuleWrap::Initialize(Local target, target->Set(FIXED_ONE_BYTE_STRING(isolate, "ModuleWrap"), tpl->GetFunction()); env->SetMethod(target, "resolve", node::loader::ModuleWrap::Resolve); + env->SetMethod(target, + "setImportModuleDynamicallyCallback", + node::loader::ModuleWrap::SetImportModuleDynamicallyCallback); } } // namespace loader diff --git a/src/module_wrap.h b/src/module_wrap.h index 980c802c1f1952..71ebda4c109b5f 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -39,6 +39,8 @@ class ModuleWrap : public BaseObject { static void GetUrl(v8::Local property, const v8::PropertyCallbackInfo& info); static void Resolve(const v8::FunctionCallbackInfo& args); + static void SetImportModuleDynamicallyCallback( + const v8::FunctionCallbackInfo& args); static v8::MaybeLocal ResolveCallback( v8::Local context, v8::Local specifier, diff --git a/test/es-module/test-esm-dynamic-import.js b/test/es-module/test-esm-dynamic-import.js new file mode 100644 index 00000000000000..a099a2ddb8a1ce --- /dev/null +++ b/test/es-module/test-esm-dynamic-import.js @@ -0,0 +1,113 @@ +// Flags: --experimental-modules --harmony-dynamic-import +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { URL } = require('url'); +const vm = require('vm'); + +common.crashOnUnhandledRejection(); + +const relativePath = './test-esm-ok.mjs'; +const absolutePath = require.resolve('./test-esm-ok.mjs'); +const targetURL = new URL('file:///'); +targetURL.pathname = absolutePath; + +function expectErrorProperty(result, propertyKey, value) { + Promise.resolve(result) + .catch(common.mustCall(error => { + assert.equal(error[propertyKey], value); + })); +} + +function expectMissingModuleError(result) { + expectErrorProperty(result, 'code', 'MODULE_NOT_FOUND'); +} + +function expectInvalidUrlError(result) { + expectErrorProperty(result, 'code', 'ERR_INVALID_URL'); +} + +function expectInvalidReferrerError(result) { + expectErrorProperty(result, 'code', 'ERR_INVALID_URL'); +} + +function expectInvalidProtocolError(result) { + expectErrorProperty(result, 'code', 'ERR_INVALID_PROTOCOL'); +} + +function expectInvalidContextError(result) { + expectErrorProperty(result, + 'message', 'import() called outside of main context'); +} + +function expectOkNamespace(result) { + Promise.resolve(result) + .then(common.mustCall(ns => { + // Can't deepStrictEqual because ns isn't a normal object + assert.deepEqual(ns, { default: true }); + })); +} + +function expectFsNamespace(result) { + Promise.resolve(result) + .then(common.mustCall(ns => { + assert.equal(typeof ns.default.writeFile, 'function'); + })); +} + +// For direct use of import expressions inside of CJS or ES modules, including +// via eval, all kinds of specifiers should work without issue. +(function testScriptOrModuleImport() { + // Importing another file, both direct & via eval + // expectOkNamespace(import(relativePath)); + expectOkNamespace(eval.call(null, `import("${relativePath}")`)); + expectOkNamespace(eval(`import("${relativePath}")`)); + expectOkNamespace(eval.call(null, `import("${targetURL}")`)); + + // Importing a built-in, both direct & via eval + expectFsNamespace(import("fs")); + expectFsNamespace(eval('import("fs")')); + expectFsNamespace(eval.call(null, 'import("fs")')); + + expectMissingModuleError(import("./not-an-existing-module.mjs")); + // TODO(jkrems): Right now this doesn't hit a protocol error because the + // module resolution step already rejects it. These arguably should be + // protocol errors. + expectMissingModuleError(import("node:fs")); + expectMissingModuleError(import('http://example.com/foo.js')); +})(); + +// vm.runInThisContext: +// * Supports built-ins, always +// * Supports imports if the script has a known defined origin +(function testRunInThisContext() { + // Succeeds because it's got an valid base url + expectFsNamespace(vm.runInThisContext(`import("fs")`, { + filename: __filename, + })); + expectOkNamespace(vm.runInThisContext(`import("${relativePath}")`, { + filename: __filename, + })); + // Rejects because it's got an invalid referrer URL. + // TODO(jkrems): Arguably the first two (built-in + absolute URL) could work + // with some additional effort. + expectInvalidReferrerError(vm.runInThisContext('import("fs")')); + expectInvalidReferrerError(vm.runInThisContext(`import("${targetURL}")`)); + expectInvalidReferrerError(vm.runInThisContext(`import("${relativePath}")`)); +})(); + +// vm.runInNewContext is currently completely unsupported, pending well-defined +// semantics for per-context/realm module maps in node. +(function testRunInNewContext() { + // Rejects because it's running in the wrong context + expectInvalidContextError( + vm.runInNewContext(`import("${targetURL}")`, undefined, { + filename: __filename, + }) + ); + + // Rejects because it's running in the wrong context + expectInvalidContextError(vm.runInNewContext(`import("fs")`, undefined, { + filename: __filename, + })); +})();