diff options
author | Jan Krems <jan.krems@groupon.com> | 2017-10-01 20:31:04 +0300 |
---|---|---|
committer | Jan Krems <jan.krems@groupon.com> | 2017-11-30 01:16:43 +0300 |
commit | 59e48329d00cd91f6836cd91bcb8aca92acac1f6 (patch) | |
tree | 333e94c37e83ed75039c56defdc02df36ece6275 | |
parent | b28af4dc26139778223fe61a886093a7d0e85c1f (diff) |
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.
PR-URL: https://github.com/nodejs/node/pull/15713
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
-rw-r--r-- | .eslintignore | 1 | ||||
-rw-r--r-- | lib/internal/loader/Loader.js | 21 | ||||
-rw-r--r-- | lib/internal/loader/ModuleWrap.js | 6 | ||||
-rw-r--r-- | lib/module.js | 1 | ||||
-rw-r--r-- | src/env.h | 1 | ||||
-rw-r--r-- | src/module_wrap.cc | 59 | ||||
-rw-r--r-- | src/module_wrap.h | 2 | ||||
-rw-r--r-- | test/es-module/test-esm-dynamic-import.js | 113 |
8 files changed, 201 insertions, 3 deletions
diff --git a/.eslintignore b/.eslintignore index dc4e023866f..669c27ce89b 100644 --- a/.eslintignore +++ b/.eslintignore @@ -2,6 +2,7 @@ lib/internal/v8.js lib/internal/v8_prof_polyfill.js lib/punycode.js test/addons/??_* +test/es-module/test-esm-dynamic-import.js test/fixtures test/message/esm_display_syntax_error.mjs tools/eslint diff --git a/lib/internal/loader/Loader.js b/lib/internal/loader/Loader.js index 49c8699771e..b8fe9ccfc61 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 c22e149925c..f0b9e2f757a 100644 --- a/lib/internal/loader/ModuleWrap.js +++ b/lib/internal/loader/ModuleWrap.js @@ -1,6 +1,9 @@ 'use strict'; -const { ModuleWrap } = internalBinding('module_wrap'); +const { + ModuleWrap, + setImportModuleDynamicallyCallback +} = 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); @@ -59,5 +62,6 @@ const createDynamicModule = (exports, url = '', evaluate) => { module.exports = { createDynamicModule, + setImportModuleDynamicallyCallback, ModuleWrap }; diff --git a/lib/module.js b/lib/module.js index f404c4317d7..4c4ceaf847f 100644 --- a/lib/module.js +++ b/lib/module.js @@ -472,6 +472,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 083c09a8134..ed7b7afc931 100644 --- a/src/env.h +++ b/src/env.h @@ -308,6 +308,7 @@ class ModuleWrap; V(internal_binding_cache_object, v8::Object) \ V(buffer_prototype_object, v8::Object) \ V(context, v8::Context) \ + V(host_import_module_dynamically_callback, v8::Function) \ V(http2ping_constructor_template, v8::ObjectTemplate) \ V(http2stream_constructor_template, v8::ObjectTemplate) \ V(inspector_console_api_object, v8::Object) \ diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 0e1f7c9eaf8..2474b362d7d 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -558,6 +558,62 @@ void ModuleWrap::Resolve(const FunctionCallbackInfo<Value>& args) { args.GetReturnValue().Set(result.FromJust().ToObject(env)); } +static MaybeLocal<Promise> ImportModuleDynamically( + Local<Context> context, + Local<v8::ScriptOrModule> referrer, + Local<String> 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<Promise::Resolver> resolver; + if (maybe_resolver.ToLocal(&resolver)) { + // TODO(jkrems): Turn into proper error object w/ code + Local<Value> error = v8::Exception::Error( + OneByteString(iso, "import() called outside of main context")); + if (resolver->Reject(context, error).IsJust()) { + return handle_scope.Escape(resolver.As<Promise>()); + } + } + return MaybeLocal<Promise>(); + } + + Local<Function> import_callback = + env->host_import_module_dynamically_callback(); + Local<Value> import_args[] = { + referrer->GetResourceName(), + Local<Value>(specifier) + }; + MaybeLocal<Value> maybe_result = import_callback->Call(context, + v8::Undefined(iso), + 2, + import_args); + + Local<Value> result; + if (maybe_result.ToLocal(&result)) { + return handle_scope.Escape(result.As<Promise>()); + } + return MaybeLocal<Promise>(); +} + +void ModuleWrap::SetImportModuleDynamicallyCallback( + const FunctionCallbackInfo<Value>& 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<Function> import_callback = args[0].As<Function>(); + env->set_host_import_module_dynamically_callback(import_callback); + + iso->SetHostImportModuleDynamicallyCallback(ImportModuleDynamically); +} + void ModuleWrap::Initialize(Local<Object> target, Local<Value> unused, Local<Context> context) { @@ -575,6 +631,9 @@ void ModuleWrap::Initialize(Local<Object> 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 a8dd89b7900..ec4d6bf5772 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -39,6 +39,8 @@ class ModuleWrap : public BaseObject { static void GetUrl(v8::Local<v8::String> property, const v8::PropertyCallbackInfo<v8::Value>& info); static void Resolve(const v8::FunctionCallbackInfo<v8::Value>& args); + static void SetImportModuleDynamicallyCallback( + const v8::FunctionCallbackInfo<v8::Value>& args); static v8::MaybeLocal<v8::Module> ResolveCallback( v8::Local<v8::Context> context, v8::Local<v8::String> 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 00000000000..a099a2ddb8a --- /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, + })); +})(); |