Skip to content

Commit

Permalink
module: Set dynamic import callback
Browse files Browse the repository at this point in the history
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: #17823
PR-URL: #15713
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
  • Loading branch information
Jan Krems authored and rvagg committed Aug 16, 2018
1 parent 656ceea commit d7192c4
Show file tree
Hide file tree
Showing 8 changed files with 202 additions and 4 deletions.
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -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
Expand Down
21 changes: 19 additions & 2 deletions lib/internal/loader/Loader.js
Original file line number Diff line number Diff line change
@@ -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');
Expand All @@ -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. */
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 6 additions & 2 deletions lib/internal/loader/ModuleWrap.js
Original file line number Diff line number Diff line change
@@ -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);
Expand Down Expand Up @@ -60,5 +63,6 @@ const createDynamicModule = (exports, url = '', evaluate) => {

module.exports = {
createDynamicModule,
setImportModuleDynamicallyCallback,
ModuleWrap
};
1 change: 1 addition & 0 deletions lib/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,7 @@ Module._load = function(request, parent, isMain) {
ESMLoader.hook(hooks);
}
}
Loader.registerImportDynamicallyCallback(ESMLoader);
await ESMLoader.import(getURLFromFilePath(request).pathname);
})()
.catch((e) => {
Expand Down
1 change: 1 addition & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand Down
59 changes: 59 additions & 0 deletions src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,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) {
Expand All @@ -567,6 +623,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
Expand Down
2 changes: 2 additions & 0 deletions src/module_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
113 changes: 113 additions & 0 deletions test/es-module/test-esm-dynamic-import.js
Original file line number Diff line number Diff line change
@@ -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,
}));
})();

0 comments on commit d7192c4

Please sign in to comment.