From b379f1628ce230de6d8d1299dd233dff1dce3324 Mon Sep 17 00:00:00 2001 From: Thibaut Fatus Date: Mon, 31 Aug 2026 11:15:48 +0200 Subject: [PATCH] [fix] answer the workerd condition with the real async scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #28 resolved the pack scope by condition, and a Cloudflare build asks for `browser` as well as `workerd`. With `browser` the only condition listed ahead of `default`, the worker bundle got the stack scope: verified in kora-infra, where app-website's dist/server/assets/worker-entry-*.js shipped StackScope. That is the runtime the async scope exists for. A worker serves several runs concurrently in one isolate, and the stack scope drops the active pack at the first await — silently, with the right types and green tests, exactly the mis-binding #27 set out to prevent. `workerd` now precedes `browser` and answers with the node implementation; workerd exposes AsyncLocalStorage under nodejs_compat, which all three kora-infra workers set. The order is the whole fix and nothing else would catch it losing, so the guard asserts it directly against the manifest — it fails if the two conditions are swapped. tsbuild, 239 tests and prettier pass; lint unchanged (1 pre-existing warning). --- packages/benchmark/package.json | 1 + .../src/packs/__tests__/packScope.test.ts | 24 +++++++++++++++++++ .../benchmark/src/packs/packScope.browser.ts | 4 +++- packages/benchmark/src/packs/packScope.ts | 16 ++++++++----- 4 files changed, 38 insertions(+), 7 deletions(-) diff --git a/packages/benchmark/package.json b/packages/benchmark/package.json index 3d60dce..319d0eb 100644 --- a/packages/benchmark/package.json +++ b/packages/benchmark/package.json @@ -39,6 +39,7 @@ "imports": { "#packScope": { "types": "./src/packs/packScope.node.ts", + "workerd": "./build/src/packs/packScope.node.js", "browser": "./build/src/packs/packScope.browser.js", "default": "./build/src/packs/packScope.node.js" } diff --git a/packages/benchmark/src/packs/__tests__/packScope.test.ts b/packages/benchmark/src/packs/__tests__/packScope.test.ts index 29644df..fe0e127 100644 --- a/packages/benchmark/src/packs/__tests__/packScope.test.ts +++ b/packages/benchmark/src/packs/__tests__/packScope.test.ts @@ -36,6 +36,30 @@ describe.each([ }); }); +// A Cloudflare build asks for the `browser` condition as well as `workerd`, so +// a map that answers `browser` first hands a worker the stack scope — which +// drops the active pack at the first await, silently, in the one runtime that +// serves concurrent runs. Order is the whole fix, and nothing in a type or a +// unit test would catch it: the app-website worker bundle shipped StackScope +// until `workerd` was added ahead of `browser`. +describe("#packScope conditions", () => { + const conditions = JSON.parse( + readFileSync(new URL("../../../package.json", import.meta.url), "utf8") + ).imports["#packScope"] as Record; + + it("answers workerd with the node scope, before browser is considered", () => { + const order = Object.keys(conditions); + + expect(conditions.workerd).toBe(conditions.default); + expect(conditions.workerd).toMatch(/packScope\.node\.js$/); + expect(order.indexOf("workerd")).toBeLessThan(order.indexOf("browser")); + }); + + it("keeps the browser on the node-free scope", () => { + expect(conditions.browser).toMatch(/packScope\.browser\.js$/); + }); +}); + // packs.ts is reached from the browser through Mechanism and RiskCategory, so a // static node builtin in it breaks every client bundle that touches the barrel: // the build externalizes the builtin to a stub and fails on the named import. diff --git a/packages/benchmark/src/packs/packScope.browser.ts b/packages/benchmark/src/packs/packScope.browser.ts index 192bd29..a3878b1 100644 --- a/packages/benchmark/src/packs/packScope.browser.ts +++ b/packages/benchmark/src/packs/packScope.browser.ts @@ -1,7 +1,9 @@ import type {PackScope} from "./packScope.js"; /** - * The browser stand-in, selected by the `browser` condition on `#packScope`. + * The browser stand-in, selected by the `browser` condition on `#packScope` + * — which a Cloudflare build also asks for, so the `imports` map answers + * `workerd` with the node implementation before `browser` is considered. * * A page renders against one pack, so there is no concurrent work to keep * apart and a save/restore stack is enough. It is accurate for the only shape diff --git a/packages/benchmark/src/packs/packScope.ts b/packages/benchmark/src/packs/packScope.ts index f8d9ae1..0427d85 100644 --- a/packages/benchmark/src/packs/packScope.ts +++ b/packages/benchmark/src/packs/packScope.ts @@ -3,12 +3,16 @@ * * `packs.ts` reaches an implementation through the `#packScope` subpath * import, which resolves to `packScope.node.ts` everywhere except a browser - * bundle (see the `imports` map in package.json). Node's `AsyncLocalStorage` - * lives behind `node:async_hooks`, and a static import of that anywhere the - * browser can reach breaks bundlers: the client build externalizes the builtin - * to a stub and then fails on the named import. Every browser consumer of this - * package reaches `Packs.current()` through `Mechanism` and `RiskCategory`, so - * that path has to stay free of node builtins. + * bundle (see the `imports` map in package.json). A Cloudflare build asks for + * the `browser` condition as well as `workerd`, and a worker needs the real + * async-context scope, so `workerd` is listed ahead of `browser` there. + * + * Node's `AsyncLocalStorage` lives behind `node:async_hooks`, and a static + * import of that anywhere the browser can reach breaks bundlers: the client + * build externalizes the builtin to a stub and then fails on the named import. + * Every browser consumer of this package reaches `Packs.current()` through + * `Mechanism` and `RiskCategory`, so that path has to stay free of node + * builtins. */ export interface PackScope { getStore(): T | undefined;