fix(jsc): AsyncDisposableStack.use() with sync @@dispose fallback#27168
fix(jsc): AsyncDisposableStack.use() with sync @@dispose fallback#27168
Conversation
…4277) Add regression test for AsyncDisposableStack.use() throwing "@@asyncDispose must be callable" when passed an object that only has Symbol.dispose (not Symbol.asyncDispose). The fix is in WebKit's getAsyncDisposableMethod builtin which needs to check for undefined/null before isCallable, then fall back to @@dispose per the TC39 spec. See oven-sh/WebKit#162. Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Comment |
|
✅ 99b36 — Looks good! Reviewed 1 file in |
Summary
Fixes #24277
AsyncDisposableStack.prototype.use()throwingTypeError: @@asyncDispose must be callablewhen passed an object with onlySymbol.dispose(notSymbol.asyncDispose)Root cause
The
getAsyncDisposableMethodJSC builtin function had two bugs:!isCallable(method)beforeisUndefinedOrNull(method), causing a TypeError when@@asyncDisposewasundefinedinstead of falling through to the undefined check@@disposeentirely — per the TC39 spec (GetDisposeMethodwith hintasync-dispose), if@@asyncDisposeis not present, the runtime should fall back to@@disposeand wrap it in a Promise adapterAdditionally,
AsyncDisposableStackPrototype.use()was using@getDisposableStackInternalField(sync) instead of@getAsyncDisposableStackInternalField(async).What this PR adds
A regression test that validates:
AsyncDisposableStack.use()with sync@@disposefalls back correctlyawait usingwith sync@@disposeworks correctly@@asyncDisposenor@@disposestill throw TypeError@@asyncDisposecontinue to work as beforeWebKit dependency
This PR depends on the WebKit fix in oven-sh/WebKit#162 being merged and a new WebKit release being created with the updated commit hash in
cmake/tools/SetupWebKit.cmake.Test plan
WEBKIT_LOCAL=ON)🤖 Generated with Claude Code