[fix] answer the workerd condition with the real async scope - #29
Merged
Merged
Conversation
#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
#28 resolved the pack scope by condition, and a Cloudflare build asks for
browseras well asworkerd. Withbrowserthe only condition ahead ofdefault, the worker bundle got the stack scope. Verified in kora-infra —app-website/dist/server/assets/worker-entry-*.jsshipped: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, which is exactly the mis-binding #27 set out to prevent.
Caught by running the infra website locally and reading the built bundles, not by any check: the build succeeds either way.
What
workerdnow precedesbrowserand answers with the node implementation. workerd exposesAsyncLocalStorageundernodejs_compat, which all three kora-infra workers set.The order is the 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 (checked by swapping them).
Verified downstream
After mirroring the map in kora-infra:
app-website/dist/server— noStackScope;createPackScope()returnsnew AsyncLocalStorage()fromnode:async_hooksapp-website/dist/client— still noasync_hooksreferenceworker-engine(wrangler deploy --dry-run) — same,new AsyncLocalStorage()tsbuild, 239 tests and prettier pass; lint unchanged (1 pre-existing warning).