Skip to content

Commit

Permalink
More incremental impl of new module registry
Browse files Browse the repository at this point in the history
Starts adding api tests, fixing up some behaviors.
  • Loading branch information
jasnell committed Sep 10, 2024
1 parent da2d274 commit 9f774c4
Show file tree
Hide file tree
Showing 8 changed files with 300 additions and 100 deletions.
6 changes: 6 additions & 0 deletions src/workerd/api/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -534,3 +534,9 @@ wd_test(
"//conditions:default": ["@platforms//:incompatible"],
}),
)

wd_test(
src = "tests/new-module-registry-test.wd-test",
args = ["--experimental"],
data = ["tests/new-module-registry-test.js"],
)
141 changes: 141 additions & 0 deletions src/workerd/api/tests/new-module-registry-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
import { notStrictEqual, ok, rejects, strictEqual } from 'assert';
import { foo, default as def } from 'foo';
import { default as fs } from 'node:fs';
const { foo: foo2, default: def2 } = await import('bar');

// Verify that import.meta.url is correct here.
strictEqual(import.meta.url, 'file:///worker');

// Verify that import.meta.main is true here.
ok(import.meta.main);

// Verify that import.meta.resolve provides correct results here.
// The input should be interpreted as a URL and normalized according
// to the rules in the WHATWG URL specification.
strictEqual(import.meta.resolve('./.././test/.././../foo'), 'file:///foo');

// There are four tests at this top level... one for the import of the node:assert
// module without the node: prefix specifier, two for the imports of the foo and
// bar modules from the worker, and one for the aliases node:fs module from the
// module worker.

strictEqual(foo, 1);
strictEqual(def, 2);
strictEqual(foo2, 1);
strictEqual(def2, 2);
strictEqual(fs, 'abc');

// Equivalent to the above, but using the file: URL scheme.
import { foo as foo3, default as def3 } from 'file:///foo';
strictEqual(foo, foo3);
strictEqual(def, def3);

// Verify that a module is unable to perform IO operations at the top level, even if
// the dynamic import is initiated within the scope of an active IoContext.
export const noTopLevelIo = {
async test() {
await rejects(import('bad'), {
message: /^Disallowed operation called within global scope/,
});
},
};

// Verify that async local storage is propagated into dynamic imports.
export const alsPropagationDynamicImport = {
async test() {
const { AsyncLocalStorage } = await import('async_hooks');
globalThis.als = new AsyncLocalStorage();
const res = await globalThis.als.run(123, () => import('als'));
strictEqual(res.default, 123);
},
};

// Query strings and fragments create new instances of known modules.
export const queryAndFragment = {
async test() {
// Each resolves the same underlying module but creates a new instance.
// The exports should be the same but the module namespaces should be different.

const a = await import('foo?query');
const b = await import('foo#fragment');
const c = await import('foo?query#fragment');
const d = await import('foo');

strictEqual(a.default, 2);
strictEqual(a.foo, 1);
strictEqual(a.default, b.default);
strictEqual(a.default, c.default);
strictEqual(a.default, d.default);
strictEqual(a.foo, b.foo);
strictEqual(a.foo, c.foo);
strictEqual(a.foo, d.foo);

notStrictEqual(a, b);
notStrictEqual(a, c);
notStrictEqual(a, d);
notStrictEqual(b, c);
notStrictEqual(b, d);
notStrictEqual(c, d);

// The import.meta.url for each should match the specifier used to import the instance.
strictEqual(a.bar, 'file:///foo?query');
strictEqual(b.bar, 'file:///foo#fragment');
strictEqual(c.bar, 'file:///foo?query#fragment');
strictEqual(d.bar, 'file:///foo');
},
};

// We do not currently support import assertions/attributes. Per the recommendation
// in the spec, we throw an error when they are encountered.
export const importAssertionsFail = {
async test() {
await rejects(import('ia'), {
message: /^Import attributes are not supported/,
});
await rejects(import('foo', { with: { a: 'abc' } }), {
message: /^Import attributes are not supported/,
});
},
};

export const invalidUrlAsSpecifier = {
async test() {
await rejects(import('zebra: not a \x00 valid URL'), {
message: /Module not found/,
});
},
};

// TODO(now): Tests
// * [ ] Include tests for all known module types
// * [ ] ESM
// * [ ] CommonJS
// * [ ] Text
// * [ ] Data
// * [ ] JSON
// * [ ] WASM
// * [ ] Python
// * [x] IO is forbidden in top-level module scope
// * [x] Async local storage context is propagated into dynamic imports
// * [ ] require(...) Works in CommonJs Modules
// * [x] Static import correctly handles node: modules with/without the node: prefix
// * [x] Dynamic import correctly handles node: modules with/without the node: prefix
// * [x] Worker bundle can alias node: modules
// * [ ] require(...) correctly handles node: modules with/without the node: prefix
// * [ ] modules not found are correctly reported as errors
// * [ ] Circular dependencies are correctly handled
// * [ ] Errors during ESM evaluation are correctly reported
// * [ ] Errors during CommonJs evaluation are correctly reported
// * [ ] Errors during dynamic import are correctly reported
// * [ ] Errors in JSON module parsing are correctly reported
// * [ ] ESM with no default export is correctly reported as error
// * [ ] CommonJs modules correctly expose named exports
// * [ ] require('module').createRequire API works as expected
// * [ ] Fallback service works as expected
// * [x] Module specifiers are correctly handled as URLs
// * [x] Querys and fragments resolve new instances of known modules
// * [x] URL resolution works correctly
// * [x] Invalid URLs are correctly reported as errors
// * [x] Import assertions should be rejected
// * [ ] console.log output correctly uses node-internal:inspect for output
// ...
32 changes: 32 additions & 0 deletions src/workerd/api/tests/new-module-registry-test.wd-test
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
using Workerd = import "/workerd/workerd.capnp";

const unitTests :Workerd.Config = (
services = [
( name = "new-module-registry-test",
worker = (
modules = [
(name = "worker", esModule = embed "new-module-registry-test.js"),
(name = "foo", esModule = "export const foo = 1; export default 2; export const bar = import.meta.url"),
(name = "bar", esModule = "export const foo = 1; export default 2;"),
(name = "node:fs", esModule = "export default 'abc'"),

# Intentionally bad module to test error handling.
# Evaluation will error because i/o is not permitted at top-level scope.
(name = "bad", esModule = "export default 1; setTimeout(() => {}, 10)"),

# Ensure that async context is propagated into a dynamic import.
(name = "als", esModule = "export default globalThis.als.getStore()"),

# Import assertions are not supported currently
(name = "ia", esModule = "import * as def from 'foo' with { a: 'test' }"),
],
compatibilityDate = "2024-07-01",
compatibilityFlags = [
"nodejs_compat_v2",
"new_module_registry",
"experimental",
],
)
),
],
);
149 changes: 79 additions & 70 deletions src/workerd/jsg/modules-new.c++
Original file line number Diff line number Diff line change
Expand Up @@ -649,11 +649,9 @@ v8::MaybeLocal<v8::Promise> dynamicImport(v8::Local<v8::Context> context,
//
// For now, we do not support any import attributes, so if there are any at all
// we will reject the import.
if (!import_assertions.IsEmpty()) {
if (import_assertions->Length() > 0) {
return rejected(js, js.typeError("Import attributes are not supported"));
};
}
if (!import_assertions.IsEmpty() && import_assertions->Length() > 0) {
return rejected(js, js.typeError("Import attributes are not supported"));
};

Url referrer = ([&] {
if (resource_name.IsEmpty()) {
Expand All @@ -663,6 +661,14 @@ v8::MaybeLocal<v8::Promise> dynamicImport(v8::Local<v8::Context> context,
return KJ_ASSERT_NONNULL(Url::tryParse(str.asPtr()));
})();

// If Node.js Compat v2 mode is enable, we have to check to see if the specifier
// is a bare node specifier and resolve it to a full node: URL.
if (isNodeJsCompatEnabled(js)) {
KJ_IF_SOME(nodeSpec, checkNodeSpecifier(spec)) {
spec = kj::mv(nodeSpec);
}
}

KJ_IF_SOME(url, referrer.tryResolve(spec.asPtr())) {
auto normalized = url.clone(Url::EquivalenceOption::NORMALIZE_PATH);
auto& registry = IsolateModuleRegistry::from(isolate);
Expand Down Expand Up @@ -691,7 +697,6 @@ IsolateModuleRegistry::IsolateModuleRegistry(
auto isolate = js.v8Isolate;
auto context = isolate->GetCurrentContext();
KJ_ASSERT(!context.IsEmpty());
KJ_ASSERT(context->GetAlignedPointerFromEmbedderData(2) == nullptr);
context->SetAlignedPointerInEmbedderData(2, this);
isolate->SetHostImportModuleDynamicallyCallback(&dynamicImport);
isolate->SetHostInitializeImportMetaObjectCallback(&importMeta);
Expand All @@ -706,76 +711,80 @@ v8::MaybeLocal<v8::Module> resolveCallback(v8::Local<v8::Context> context,
auto& registry = IsolateModuleRegistry::from(isolate);
auto& js = Lock::from(isolate);

try {
return js.tryCatch([&]() -> v8::MaybeLocal<v8::Module> {
auto spec = kj::str(specifier);

// The proposed specification for import assertions strongly recommends that
// embedders reject import attributes and types they do not understand/implement.
// This is because import attributes can alter the interpretation of a module.
// Throwing an error for things we do not understand is the safest thing to do
// for backwards compatibility.
//
// For now, we do not support any import attributes, so if there are any at all
// we will reject the import.
if (!import_assertions.IsEmpty()) {
if (import_assertions->Length() > 0) {
js.throwException(js.typeError("Import attributes are not supported"));
};
}
return js.tryCatch([&]() -> v8::MaybeLocal<v8::Module> {
auto spec = kj::str(specifier);

// The proposed specification for import assertions strongly recommends that
// embedders reject import attributes and types they do not understand/implement.
// This is because import attributes can alter the interpretation of a module.
// Throwing an error for things we do not understand is the safest thing to do
// for backwards compatibility.
//
// For now, we do not support any import attributes, so if there are any at all
// we will reject the import.
if (!import_assertions.IsEmpty()) {
if (import_assertions->Length() > 0) {
js.throwException(js.typeError("Import attributes are not supported"));
};
}

ResolveContext::Type type = ResolveContext::Type::BUNDLE;
ResolveContext::Type type = ResolveContext::Type::BUNDLE;

auto& referrerUrl = registry.lookup(js, referrer)
.map([&](IsolateModuleRegistry::Entry& entry) -> const Url& {
switch (entry.module.type()) {
case Module::Type::BUNDLE: {
type = ResolveContext::Type::BUNDLE;
break;
}
case Module::Type::BUILTIN: {
type = ResolveContext::Type::BUILTIN;
break;
}
case Module::Type::BUILTIN_ONLY: {
type = ResolveContext::Type::BUILTIN_ONLY;
break;
}
case Module::Type::FALLBACK: {
type = ResolveContext::Type::BUNDLE;
break;
}
auto& referrerUrl = registry.lookup(js, referrer)
.map([&](IsolateModuleRegistry::Entry& entry) -> const Url& {
switch (entry.module.type()) {
case Module::Type::BUNDLE: {
type = ResolveContext::Type::BUNDLE;
break;
}
return entry.specifier.specifier;
}).orDefault(ModuleBundle::BundleBuilder::BASE);
case Module::Type::BUILTIN: {
type = ResolveContext::Type::BUILTIN;
break;
}
case Module::Type::BUILTIN_ONLY: {
type = ResolveContext::Type::BUILTIN_ONLY;
break;
}
case Module::Type::FALLBACK: {
type = ResolveContext::Type::BUNDLE;
break;
}
}
return entry.specifier.specifier;
}).orDefault(ModuleBundle::BundleBuilder::BASE);

KJ_IF_SOME(url, referrerUrl.tryResolve(spec)) {
// Make sure that percent-encoding in the path is normalized so we can match correctly.
auto normalized = url.clone(Url::EquivalenceOption::NORMALIZE_PATH);
ResolveContext resolveContext = {
.type = type,
.source = ResolveContext::Source::STATIC_IMPORT,
.specifier = normalized,
.referrer = referrerUrl,
.rawSpecifier = spec.asPtr(),
};
// TODO(soon): Add import assertions to the context.

return registry.resolve(js, resolveContext);
// If Node.js Compat v2 mode is enable, we have to check to see if the specifier
// is a bare node specifier and resolve it to a full node: URL.
if (isNodeJsCompatEnabled(js)) {
KJ_IF_SOME(nodeSpec, checkNodeSpecifier(spec)) {
spec = kj::mv(nodeSpec);
}
}

js.throwException(js.error(kj::str("Invalid module specifier: "_kj, specifier)));
}, [&](Value exception) -> v8::MaybeLocal<v8::Module> {
// If there are any synchronously thrown exceptions, we want to catch them
// here and convert them into a rejected promise. The only exception are
// fatal cases where the isolate is terminating which won't make it here
// anyway.
js.v8Isolate->ThrowException(exception.getHandle(js));
return v8::MaybeLocal<v8::Module>();
});
} catch (...) {
kj::throwFatalException(kj::getCaughtExceptionAsKj());
}
KJ_IF_SOME(url, referrerUrl.tryResolve(spec)) {
// Make sure that percent-encoding in the path is normalized so we can match correctly.
auto normalized = url.clone(Url::EquivalenceOption::NORMALIZE_PATH);
ResolveContext resolveContext = {
.type = type,
.source = ResolveContext::Source::STATIC_IMPORT,
.specifier = normalized,
.referrer = referrerUrl,
.rawSpecifier = spec.asPtr(),
};
// TODO(soon): Add import assertions to the context.

return registry.resolve(js, resolveContext);
}

js.throwException(js.error(kj::str("Invalid module specifier: "_kj, specifier)));
}, [&](Value exception) -> v8::MaybeLocal<v8::Module> {
// If there are any synchronously thrown exceptions, we want to catch them
// here and convert them into a rejected promise. The only exception are
// fatal cases where the isolate is terminating which won't make it here
// anyway.
js.v8Isolate->ThrowException(exception.getHandle(js));
return v8::MaybeLocal<v8::Module>();
});
}

// The fallback module bundle calls a single resolve callback to resolve all modules
Expand Down
Loading

0 comments on commit 9f774c4

Please sign in to comment.