Skip to content

fix(shared): make await using work without a native Symbol.asyncDispose - #156

Merged
dinwwwh merged 2 commits into
mainfrom
claude/dreamy-thompson-q5uqav
Oct 12, 2026
Merged

dinwwwh merged 2 commits into
mainfrom
claude/dreamy-thompson-q5uqav

Conversation

@dinwwwh

@dinwwwh dinwwwh commented Oct 12, 2026

Copy link
Copy Markdown
Member

Summary

Updated the fallback symbol key for asyncDispose to use Symbol.for('Symbol.asyncDispose') instead of Symbol.for('asyncDispose'), aligning with the standard used by esbuild, Babel, and core-js.

Key Changes

  • Changed fallback symbol from Symbol.for('asyncDispose') to Symbol.for('Symbol.asyncDispose') to match ecosystem conventions
  • Updated JSDoc comment to clarify the fallback behavior
  • Removed unnecessary v8 ignore comments around the symbol initialization
  • Added comprehensive test case verifying the fallback behavior when Symbol.asyncDispose is not natively available

Implementation Details

The fallback symbol is now consistent with how other major JavaScript tooling and libraries handle the asyncDispose symbol on runtimes that don't have native support. The test uses a Proxy to simulate an environment where Symbol.asyncDispose is undefined, ensuring the fallback mechanism works correctly and calls cleanup with the expected { kind: 'cancelled' } parameter.

https://claude.ai/code/session_01NRRM1bujEJiPEv7FbiLXDr

…se fallback

On runtimes without a native Symbol.asyncDispose (Hermes/React Native,
older Safari or Node), AsyncIteratorClass registered its dispose method
under Symbol.for('asyncDispose'). esbuild's `__using`, Babel's `usingCtx`
and core-js all look it up under Symbol.for('Symbol.asyncDispose'), so
`await using` on an event-stream body threw "Object not disposable" and
its cleanup never ran.

Switch the fallback to the conventional key (still preferring the native
symbol), correct the JSDoc, and cover the fallback path with a test so the
v8 ignore markers can go.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NRRM1bujEJiPEv7FbiLXDr
Take onTestFinished from the test context like the rest of the repo, drop
a comment that repeats the one in iterator.ts, and call the dispose method
directly instead of checking its type and invoking it via .call.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NRRM1bujEJiPEv7FbiLXDr
@dinwwwh dinwwwh changed the title Fix asyncDispose fallback symbol to match ecosystem standards fix(shared): make await using work without a native Symbol.asyncDispose Oct 12, 2026
@pkg-pr-new

pkg-pr-new Bot commented Oct 12, 2026

Copy link
Copy Markdown
@standard-server/aws-lambda

npm i https://pkg.pr.new/@standard-server/aws-lambda@156

@standard-server/core

npm i https://pkg.pr.new/@standard-server/core@156

@standard-server/fastify

npm i https://pkg.pr.new/@standard-server/fastify@156

@standard-server/fetch

npm i https://pkg.pr.new/@standard-server/fetch@156

@standard-server/node

npm i https://pkg.pr.new/@standard-server/node@156

@standard-server/peer

npm i https://pkg.pr.new/@standard-server/peer@156

@standard-server/shared

npm i https://pkg.pr.new/@standard-server/shared@156

commit: 8a8e59b

@codecov

codecov Bot commented Oct 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed

codspeed Bot commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

⚠️ 5 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

✅ 26 untouched benchmarks
⏩ 108 skipped benchmarks1


Comparing claude/dreamy-thompson-q5uqav (8a8e59b) with main (6c1cc3b)

Open in CodSpeed

Footnotes

  1. 108 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — initial review of PR #156 (8a8e59b against base 6c1cc3b).

  • Fallback symbol key corrected (packages/shared/src/iterator.ts) — the no-native-symbol fallback now uses Symbol.for('Symbol.asyncDispose') instead of Symbol.for('asyncDispose'), matching the key esbuild's __knownSymbol('asyncDispose'), Babel's usingCtx, and core-js's well-known-symbol polyfill all use.
  • Comment/JSDoc updated — the stale "available in esnext" note is replaced with an accurate native-vs-fallback description; the two /* v8 ignore */ markers are dropped now that the fallback branch is exercised.
  • Regression test added (packages/shared/src/iterator.test.ts) — stubs a Symbol proxy whose asyncDispose is undefined, re-imports the module, and asserts disposal dispatches under Symbol.for('Symbol.asyncDispose') with { kind: 'cancelled' }.

Verified locally against head 8a8e59b: packages/shared/src/iterator.test.ts passes (22 tests), iterator.ts reports 100% line/branch/function coverage, and pnpm run check (sherif, oxlint, oxfmt, tsc) is clean. I confirmed the fallback key against the actual upstream sources — esbuild's __using uses __knownSymbol('asyncDispose') → Symbol.for('Symbol.asyncDispose'), Babel's usingCtx uses Symbol.asyncDispose || Symbol.for("Symbol.asyncDispose"), and core-js resolves well-known symbols to Symbol.for('Symbol.' + name) without native support. This closes a real interop gap for await using on runtimes without a native Symbol.asyncDispose, while leaving the native path unchanged.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@dinwwwh
dinwwwh merged commit 7523a6f into main Oct 12, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants