Skip to content

Commit

Permalink
Disable top-level await with a compat flag
Browse files Browse the repository at this point in the history
Currently, the top level scope of a Worker script and modules
that are imported may use top level await. However, because we
restrict the use of i/o in the top level scope, the only thing
that can be awaited at the top level scope is a dynamic import,
which we require to be resolved synchronously. It is a bit
nonsensical to support synchronously resolved dynamic imports
at the top level when we can just use static imports more
effectively. Additionally, the ecosystem and runtimes are moving
to a state where top level await in modules is being strongly
discouraged. This compat flag. This compatibility flag will
make it impossible to use top level await.
  • Loading branch information
jasnell committed Nov 8, 2024
1 parent df6d46c commit 0e60984
Show file tree
Hide file tree
Showing 11 changed files with 87 additions and 16 deletions.
13 changes: 11 additions & 2 deletions src/workerd/api/node/module.c++
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// https://opensource.org/licenses/Apache-2.0
#include "module.h"

#include <workerd/io/features.h>
#include <workerd/jsg/url.h>

namespace workerd::api::node {
Expand Down Expand Up @@ -88,11 +89,19 @@ jsg::JsValue ModuleUtil::createRequire(jsg::Lock& js, kj::String path) {
auto handle = jsg::check(module->Evaluate(js.v8Context()));
KJ_ASSERT(handle->IsPromise());
auto prom = handle.As<v8::Promise>();
if (prom->State() == v8::Promise::PromiseState::kPending) {

if (module->IsGraphAsync()) {
// There's a top level await in the module. If TLA is enabled, we need to run the
// microtasks. If TLA is disabled, we need to throw an error.

JSG_REQUIRE(
FeatureFlags::get(js).getNoTopLevelAwait(), Error, "Top-level await is not permitted");
js.runMicrotasks();
}

JSG_REQUIRE(prom->State() != v8::Promise::PromiseState::kPending, Error,
"Module evaluation did not complete synchronously.");
"Top-level await in module is unsettled");

if (module->GetStatus() == v8::Module::kErrored) {
jsg::throwTunneledException(js.v8Isolate, module->GetException());
}
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/node/tests/module-create-require-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export const doTheTest = {
strictEqual(assert, required);

throws(() => require('invalid'), {
message: 'Module evaluation did not complete synchronously.',
message: 'Top-level await in module is unsettled',
});

throws(() => require('does not exist'));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const unitTests :Workerd.Config = (
(name = "invalid", esModule = "const p = new Promise(() => {}); await p;"),
],
compatibilityDate = "2024-08-01",
compatibilityFlags = ["nodejs_compat_v2"]
compatibilityFlags = ["enable_top_level_await", "nodejs_compat_v2"]
)
),
],
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/tests/new-module-registry-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
import { foo, default as def } from 'foo';
import { default as fs } from 'node:fs';
import { Buffer } from 'buffer';
const { foo: foo2, default: def2 } = await import('bar');
import { foo as foo2, default as def2 } from 'bar';

// Verify that import.meta.url is correct here.
strictEqual(import.meta.url, 'file:///worker');
Expand Down
1 change: 1 addition & 0 deletions src/workerd/api/tests/new-module-registry-test.wd-test
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const unitTests :Workerd.Config = (
"nodejs_compat_v2",
"new_module_registry",
"experimental",
"disable_top_level_await",
],
)
),
Expand Down
13 changes: 13 additions & 0 deletions src/workerd/io/compatibility-date.capnp
Original file line number Diff line number Diff line change
Expand Up @@ -658,4 +658,17 @@ struct CompatibilityFlags @0x8f8c1b68151b6cef {
#
# This is a compat flag so that we can opt in our test workers into it before rolling out to
# everyone.

noTopLevelAwait @67 :Bool
$compatEnableFlag("disable_top_level_await")
$compatDisableFlag("enable_top_level_await")
$compatEnableDate("2024-12-02");
# Currently, the top level scope of a Worker script and modules that are imported may
# use top level await. However, because we restrict the use of i/o in the top level
# scope, the only thing that can be awaited at the top level scope is a dynamic import,
# which we require to be resolved synchronously. It is a bit nonsensical to support
# synchronously resolved dynamic imports at the top level when we can just use static
# imports more effectively. Additionally, the ecosystem and runtimes are moving to a
# state where top level await in modules is being strongly discouraged. This compat
# flag. This compatibility flag will make it impossible to use top level await.
}
3 changes: 3 additions & 0 deletions src/workerd/io/worker.c++
Original file line number Diff line number Diff line change
Expand Up @@ -999,6 +999,9 @@ Worker::Isolate::Isolate(kj::Own<Api> apiParam,
if (features.getNodeJsCompatV2()) {
lock->setNodeJsCompatEnabled();
}
if (!features.getNoTopLevelAwait()) {
lock->enableTopLevelAwait();
}

if (impl->inspector != kj::none || ::kj::_::Debug::shouldLog(::kj::LogSeverity::INFO)) {
lock->setLoggerCallback([this](jsg::Lock& js, kj::StringPtr message) {
Expand Down
4 changes: 4 additions & 0 deletions src/workerd/jsg/jsg.c++
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,10 @@ void Lock::setNodeJsCompatEnabled() {
IsolateBase::from(v8Isolate).setNodeJsCompatEnabled({}, true);
}

void Lock::enableTopLevelAwait() {
IsolateBase::from(v8Isolate).enableTopLevelAwait();
}

void Lock::setToStringTag() {
IsolateBase::from(v8Isolate).enableSetToStringTag();
}
Expand Down
1 change: 1 addition & 0 deletions src/workerd/jsg/jsg.h
Original file line number Diff line number Diff line change
Expand Up @@ -2516,6 +2516,7 @@ class Lock {

void setNodeJsCompatEnabled();
void setToStringTag();
void enableTopLevelAwait();

using Logger = void(Lock&, kj::StringPtr);
void setLoggerCallback(kj::Function<Logger>&& logger);
Expand Down
53 changes: 42 additions & 11 deletions src/workerd/jsg/modules.c++
Original file line number Diff line number Diff line change
Expand Up @@ -272,19 +272,29 @@ v8::Local<v8::Value> CommonJsModuleContext::require(jsg::Lock& js, kj::String sp
// Adding imported from suffix here not necessary like it is for resolveCallback, since we have a
// js stack that will include the parent module's name and location of the failed require().

JSG_REQUIRE_NONNULL(
info.maybeSynthetic, TypeError, "Cannot use require() to import an ES Module.");

auto module = info.module.getHandle(js);

check(module->InstantiateModule(js.v8Context(), &resolveCallback));
jsg::instantiateModule(js, module);

auto handle = check(module->Evaluate(js.v8Context()));
KJ_ASSERT(handle->IsPromise());
auto prom = handle.As<v8::Promise>();

// This assert should always pass since evaluateSyntheticModuleCallback() for CommonJS
// modules (below) always returns an already-resolved promise.
KJ_ASSERT(prom->State() != v8::Promise::PromiseState::kPending);
// If top-level await is disabled, then we can allow using require to import ES Modules
// since those will always be expected to resolve synchronously. If top-level await is
// permitted, we have to restrict since the module evaluation cannot complete. That said,
// if the module graph is not async, it'll be ok.
auto& isolateBase = IsolateBase::from(js.v8Isolate);
if (isolateBase.isTopLevelAwaitEnabled() && module->IsGraphAsync()) {
// The ES Module has a top-level await meaning the instantiation is not synchronous.
// We cannot support these right now. If the module does not use top-level await,
// then the promise will be immediately settled and we're fine!
JSG_REQUIRE_NONNULL(info.maybeSynthetic, TypeError,
"Cannot use require() to import an ES Module that uses top-level await.");
}

JSG_REQUIRE(prom->State() != v8::Promise::PromiseState::kPending, Error,
"Top-level await in module is unsettled");

if (module->GetStatus() == v8::Module::kErrored) {
throwTunneledException(js.v8Isolate, module->GetException());
Expand Down Expand Up @@ -341,13 +351,20 @@ void instantiateModule(jsg::Lock& js, v8::Local<v8::Module>& module) {

jsg::check(module->InstantiateModule(context, &resolveCallback));
auto prom = jsg::check(module->Evaluate(context)).As<v8::Promise>();
js.runMicrotasks();

if (module->IsGraphAsync()) {
// There's a top level await in the module. If TLA is enabled, we need to run the
// microtasks. If TLA is disabled, we need to throw an error.

auto& isolateBase = IsolateBase::from(isolate);
JSG_REQUIRE(isolateBase.isTopLevelAwaitEnabled(), Error, "Top-level await is not permitted");
js.runMicrotasks();
}

switch (prom->State()) {
case v8::Promise::kPending:
// Let's make sure nobody is depending on pending modules that do not resolve first.
KJ_LOG(WARNING, "Async module was not immediately resolved.");
break;
JSG_FAIL_REQUIRE(Error, "Top-level await in module is unsettled");
case v8::Promise::kRejected:
// Since we don't actually support I/O when instantiating a worker, we don't return the
// promise from module->Evaluate, which means we lose any errors that happen during
Expand Down Expand Up @@ -600,9 +617,23 @@ v8::Local<v8::Value> NodeJsModuleContext::require(jsg::Lock& js, kj::String spec
KJ_ASSERT(handle->IsPromise());
auto prom = handle.As<v8::Promise>();

// If top-level await is disabled, then we can allow using require to import ES Modules
// since those will always be expected to resolve synchronously. If top-level await is
// permitted, we have to restrict since the module evaluation cannot complete. That said,
// if the module graph is not async, it'll be ok.
auto& isolateBase = IsolateBase::from(js.v8Isolate);
if (isolateBase.isTopLevelAwaitEnabled() && module->IsGraphAsync()) {
// The ES Module has a top-level await meaning the instantiation is not synchronous.
// We cannot support these right now. If the module does not use top-level await,
// then the promise will be immediately settled and we're fine!
JSG_REQUIRE_NONNULL(info.maybeSynthetic, TypeError,
"Cannot use require() to import an ES Module that uses top-level await.");
}

// This assert should always pass since evaluateSyntheticModuleCallback() for CommonJS
// modules (below) always returns an already-resolved promise.
KJ_ASSERT(prom->State() != v8::Promise::PromiseState::kPending);
JSG_REQUIRE(prom->State() != v8::Promise::PromiseState::kPending, Error,
"Top-level await in module is unsettled");

if (module->GetStatus() == v8::Module::kErrored) {
jsg::throwTunneledException(js.v8Isolate, module->GetException());
Expand Down
9 changes: 9 additions & 0 deletions src/workerd/jsg/setup.h
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,14 @@ class IsolateBase {
setToStringTag = true;
}

inline void enableTopLevelAwait() {
allowTopLevelAwait = true;
}

inline bool isTopLevelAwaitEnabled() const {
return allowTopLevelAwait;
}

// The logger will be optionally set by the isolate setup logic if there is anywhere
// for the log to go (for instance, if debug logging is enabled or the inspector is
// being used).
Expand Down Expand Up @@ -238,6 +246,7 @@ class IsolateBase {
bool asyncContextTrackingEnabled = false;
bool nodeJsCompatEnabled = false;
bool setToStringTag = false;
bool allowTopLevelAwait = false;

kj::Maybe<kj::Function<Logger>> maybeLogger;
kj::Maybe<kj::Function<ErrorReporter>> maybeErrorReporter;
Expand Down

0 comments on commit 0e60984

Please sign in to comment.