Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vm: add support for import assertions in dynamic imports #40249

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions doc/api/vm.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ executed in specific contexts.
<!-- YAML
added: v0.3.1
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/40249
description: Added support for import assertions to the
`importModuleDynamically` parameter.
- version: v10.6.0
pr-url: https://github.com/nodejs/node/pull/20300
description: The `produceCachedData` is deprecated in favour of
Expand Down Expand Up @@ -91,6 +95,9 @@ changes:
using it in a production environment.
* `specifier` {string} specifier passed to `import()`
* `script` {vm.Script}
* `importAssertions` {Object} The `"assert"` value passed to the
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
[`optionsExpression`][] optional parameter, or an empty object if no value
was provided.
* Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is
recommended in order to take advantage of error tracking, and to avoid
issues with namespaces that contain `then` function exports.
Expand Down Expand Up @@ -642,6 +649,13 @@ The `vm.SourceTextModule` class provides the [Source Text Module Record][] as
defined in the ECMAScript specification.

### `new vm.SourceTextModule(code[, options])`
<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/40249
description: Added support for import assertions to the
`importModuleDynamically` parameter.
-->

* `code` {string} JavaScript Module code to parse
* `options`
Expand All @@ -667,6 +681,9 @@ defined in the ECMAScript specification.
`import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][].
* `specifier` {string} specifier passed to `import()`
* `module` {vm.Module}
* `importAssertions` {Object} The `"assert"` value passed to the
[`optionsExpression`][] optional parameter, or an empty object if no value
was provided.
* Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is
recommended in order to take advantage of error tracking, and to avoid
issues with namespaces that contain `then` function exports.
Expand Down Expand Up @@ -852,6 +869,10 @@ const vm = require('vm');
<!-- YAML
added: v10.10.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/40249
description: Added support for import assertions to the
`importModuleDynamically` parameter.
- version: v15.9.0
pr-url: https://github.com/nodejs/node/pull/35431
description: Added `importModuleDynamically` option again.
Expand Down Expand Up @@ -893,6 +914,9 @@ changes:
considered stable.
* `specifier` {string} specifier passed to `import()`
* `function` {Function}
* `importAssertions` {Object} The `"assert"` value passed to the
[`optionsExpression`][] optional parameter, or an empty object if no value
was provided.
* Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is
recommended in order to take advantage of error tracking, and to avoid
issues with namespaces that contain `then` function exports.
Expand Down Expand Up @@ -1068,6 +1092,10 @@ vm.measureMemory({ mode: 'detailed', execution: 'eager' })
<!-- YAML
added: v0.3.1
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/40249
description: Added support for import assertions to the
`importModuleDynamically` parameter.
- version: v6.3.0
pr-url: https://github.com/nodejs/node/pull/6635
description: The `breakOnSigint` option is supported now.
Expand Down Expand Up @@ -1113,6 +1141,9 @@ changes:
using it in a production environment.
* `specifier` {string} specifier passed to `import()`
* `script` {vm.Script}
* `importAssertions` {Object} The `"assert"` value passed to the
[`optionsExpression`][] optional parameter, or an empty object if no value
was provided.
* Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is
recommended in order to take advantage of error tracking, and to avoid
issues with namespaces that contain `then` function exports.
Expand Down Expand Up @@ -1145,6 +1176,10 @@ console.log(contextObject);
<!-- YAML
added: v0.3.1
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/40249
description: Added support for import assertions to the
`importModuleDynamically` parameter.
- version: v14.6.0
pr-url: https://github.com/nodejs/node/pull/34023
description: The `microtaskMode` option is supported now.
Expand Down Expand Up @@ -1211,6 +1246,9 @@ changes:
using it in a production environment.
* `specifier` {string} specifier passed to `import()`
* `script` {vm.Script}
* `importAssertions` {Object} The `"assert"` value passed to the
[`optionsExpression`][] optional parameter, or an empty object if no value
was provided.
* Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is
recommended in order to take advantage of error tracking, and to avoid
issues with namespaces that contain `then` function exports.
Expand Down Expand Up @@ -1247,6 +1285,10 @@ console.log(contextObject);
<!-- YAML
added: v0.3.1
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/40249
description: Added support for import assertions to the
`importModuleDynamically` parameter.
- version: v6.3.0
pr-url: https://github.com/nodejs/node/pull/6635
description: The `breakOnSigint` option is supported now.
Expand Down Expand Up @@ -1290,6 +1332,9 @@ changes:
using it in a production environment.
* `specifier` {string} specifier passed to `import()`
* `script` {vm.Script}
* `importAssertions` {Object} The `"assert"` value passed to the
[`optionsExpression`][] optional parameter, or an empty object if no value
was provided.
* Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is
recommended in order to take advantage of error tracking, and to avoid
issues with namespaces that contain `then` function exports.
Expand Down Expand Up @@ -1452,6 +1497,7 @@ are not controllable through the timeout either.
[`Error`]: errors.md#class-error
[`URL`]: url.md#class-url
[`eval()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval
[`optionsExpression`]: https://tc39.es/proposal-import-assertions/#sec-evaluate-import-call
[`script.runInContext()`]: #scriptrunincontextcontextifiedobject-options
[`script.runInThisContext()`]: #scriptruninthiscontextoptions
[`url.origin`]: url.md#urlorigin
Expand Down
10 changes: 8 additions & 2 deletions lib/internal/process/esm_loader.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
'use strict';

const {
ObjectCreate,
} = primordials;

const {
ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING,
} = require('internal/errors').codes;
Expand All @@ -22,13 +26,14 @@ exports.initializeImportMetaObject = function(wrap, meta) {
}
};

exports.importModuleDynamicallyCallback = async function(wrap, specifier) {
exports.importModuleDynamicallyCallback =
async function importModuleDynamicallyCallback(wrap, specifier, assertions) {
const { callbackMap } = internalBinding('module_wrap');
if (callbackMap.has(wrap)) {
const { importModuleDynamically } = callbackMap.get(wrap);
if (importModuleDynamically !== undefined) {
return importModuleDynamically(
specifier, getModuleFromWrap(wrap) || wrap);
specifier, getModuleFromWrap(wrap) || wrap, assertions);
}
}
throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING();
Expand Down Expand Up @@ -69,6 +74,7 @@ async function initializeLoader() {
const exports = await internalEsmLoader.import(
customLoaders,
pathToFileURL(cwd).href,
ObjectCreate(null),
);

// Hooks must then be added to external/public loader
Expand Down
2 changes: 1 addition & 1 deletion lib/vm.js
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ function compileFunction(code, params, options = {}) {
const wrapped = importModuleDynamicallyWrap(importModuleDynamically);
const func = result.function;
callbackMap.set(result.cacheKey, {
importModuleDynamically: (s, _k) => wrapped(s, func),
importModuleDynamically: (s, _k, i) => wrapped(s, func, i),
});
}

Expand Down
28 changes: 20 additions & 8 deletions src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,21 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(that);
}

static Local<Object> createImportAssertionContainer(Environment* env,
Isolate* isolate, Local<FixedArray> raw_assertions) {
Local<Object> assertions =
Object::New(isolate, v8::Null(env->isolate()), nullptr, nullptr, 0);
for (int i = 0; i < raw_assertions->Length(); i += 3) {
assertions
->Set(env->context(),
raw_assertions->Get(env->context(), i).As<String>(),
raw_assertions->Get(env->context(), i + 1).As<Value>())
.ToChecked();
}

return assertions;
}

void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Isolate* isolate = args.GetIsolate();
Expand Down Expand Up @@ -288,14 +303,7 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {

Local<FixedArray> raw_assertions = module_request->GetImportAssertions();
Local<Object> assertions =
Object::New(isolate, v8::Null(env->isolate()), nullptr, nullptr, 0);
for (int i = 0; i < raw_assertions->Length(); i += 3) {
assertions
->Set(env->context(),
raw_assertions->Get(env->context(), i).As<String>(),
raw_assertions->Get(env->context(), i + 1).As<Value>())
.ToChecked();
}
createImportAssertionContainer(env, isolate, raw_assertions);

Local<Value> argv[] = {
specifier,
Expand Down Expand Up @@ -602,9 +610,13 @@ static MaybeLocal<Promise> ImportModuleDynamically(
UNREACHABLE();
}

Local<Object> assertions =
createImportAssertionContainer(env, isolate, import_assertions);

Local<Value> import_args[] = {
object,
Local<Value>(specifier),
assertions,
};

Local<Value> result;
Expand Down
16 changes: 15 additions & 1 deletion test/parallel/test-vm-module-dynamic-import.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

// Flags: --experimental-vm-modules
// Flags: --experimental-vm-modules --harmony-import-assertions

const common = require('../common');

Expand Down Expand Up @@ -56,6 +56,20 @@ async function test() {
assert.strictEqual(foo.namespace, await globalThis.fooResult);
delete globalThis.fooResult;
}

{
const s = new Script('import("foo", { assert: { key: "value" } })', {
importModuleDynamically: common.mustCall((specifier, wrap, assertion) => {
assert.strictEqual(specifier, 'foo');
assert.strictEqual(wrap, s);
assert.deepStrictEqual(assertion, { __proto__: null, key: 'value' });
return foo;
}),
});

const result = s.runInThisContext();
assert.strictEqual(foo.namespace, await result);
}
}

async function testInvalid() {
Expand Down