[fix] resolve the pack scope per environment, not through node:async_hooks - #28
Merged
Merged
Conversation
…hooks packs.ts created its AsyncLocalStorage from a static `node:async_hooks` import. The browser reaches that module — Mechanism.listAll() and RiskCategory.listAll() both read Packs.current() — so every client bundle importing the barrel pulled a node builtin in. Bundlers externalize the builtin to a stub and then fail on the named import; kora-infra's website build has been broken since #27: "AsyncLocalStorage" is not exported by "__vite-browser-external", imported by ".../packs/packs.js" The scope now comes in through the `#packScope` subpath import, resolved by condition in package.json: - node/workers (default): packScope.node.ts, the real AsyncLocalStorage. A server runs several runs concurrently in one isolate, so the active pack still has to follow each one across its awaits. - browser: packScope.browser.ts, a save/restore stack with no node import. A page renders against one pack, and `Packs.run()` is only ever called with a synchronous callback, so the stack is accurate for that shape. Both conditions point at build output, so the specifier resolves for consumers but not for vitest running from source; the vitest config aliases it back to the node implementation, which also keeps an unbuilt checkout testable. Tests cover both implementations against the same behavior — no store outside run(), the store visible inside, and the enclosing store restored on return and on throw — plus a guard that packs.ts imports no node builtin, since nothing else would catch the regression before a downstream client build. tsbuild, 237 tests and prettier pass; lint unchanged (1 pre-existing warning).
Thibaut-Fatus
added a commit
that referenced
this pull request
Aug 31, 2026
#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).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
packs.tscreates itsAsyncLocalStoragefrom a staticnode:async_hooksimport. The browser reaches that module —Mechanism.listAll()andRiskCategory.listAll()both readPacks.current()— so every client bundle that imports the barrel pulls a node builtin in. Bundlers externalize the builtin to a stub and then fail on the named import, which has broken kora-infra's website build since #27:What
The scope now arrives through the
#packScopesubpath import, resolved by condition inpackage.json:default) —packScope.node.ts, the realAsyncLocalStorage. A server runs several runs concurrently in one isolate, so the active pack still has to follow each one across its awaits.packScope.browser.ts, a save/restore stack with no node import. A page renders against one pack, andPacks.run()is only ever called with a synchronous callback, so the stack is accurate for that shape.Both conditions point at build output, so the specifier resolves for consumers but not for vitest running from source; the vitest config aliases it back to the node implementation, which also keeps an unbuilt checkout testable.
No API change —
Packsis untouched.Tests
Both implementations run against the same behavior (no store outside
run(), store visible inside, enclosing store restored on return and on throw), plus a guard thatpacks.tsimports no node builtin — nothing else would catch the regression before a downstream client build.tsbuild, 237 tests and prettier pass; lint unchanged (1 pre-existing warning incli/reassessCommand.ts).Downstream
korabench/infra needs the matching
importsmap in itspackages/lib-benchmarkwrapper manifest; that lands with the submodule bump onfeat/pluggable-taxonomy-packs(infra #188), whereyarn app-website:buildnow exits 0 with noasync_hooksindist/clientand the real one still indist/server.