Upgrade WebKit to 2b257999 - #30705
Conversation
Upgrades the JavaScriptCore fork to upstream WebKit 49d2e914a4cc (~755 commits). JSPromise lost its JSInternalFieldObjectImpl<2> layout. The getPromiseInternalField/putPromiseInternalField bytecode intrinsics and the promiseField*/promiseState* intrinsic constants are gone. Add three C++ host functions exposed as private globals -- $peekPromiseStatus(p), $peekPromiseSettledValue(p), $pokePromiseAsHandled(p) -- and rewrite all 26 builtin call sites. Update bindings.cpp/BunPlugin.cpp to the new flags()/setFlags()/setSlot()/payloadCell() accessors. WTF replaced WyHash/SuperFastHash with RapidHash. Pull the upstream create_hash_table perl script and re-graft Bun's ConstantInteger extension so the *.lut.h string hashes match what the runtime computes; otherwise every static property on JSGlobalObject (Bun, fetch, process) is invisible. JSWebAssemblyStreamingContextType inserted at JSType slot 27 -- re-number src/jsc/JSType.rs and the .zig porting reference. compileStreaming/instantiateStreaming hooks now receive the JSPromise* instead of returning it. JSModuleNamespaceObject/AbstractModuleRecord gain a ModulePhase argument for static import defer; preserve Bun's shouldPreventExtensions parameter as a trailing default.
|
Updated 8:40 PM PT - May 16th, 2026
❌ @sosukesuzuki, your commit db90f70 has 1 failures in 🧪 To try this PR locally: bunx bun-pr 30705That installs a local version of the PR into your bun-30705 --bun |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughReplaces direct promise internal-field access with new peek/poke intrinsics and host functions, renumbers JSType to add WebAssemblyStreamingContext, refactors WebAssembly streaming APIs to operate on provided promises, updates hash-table generator to rapidhash, and migrates builtins, C++ bindings, and tooling. ChangesPromise Intrinsics and Infrastructure Refactoring
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
Found 8 issues this PR may fix:
🤖 Generated with Claude Code |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/codegen/replacements.ts`:
- Around line 259-268: The replacement for
isPromiseFulfilled/isPromiseRejected/isPromisePending produces malformed JS
because it slices inner.result with slice(0,-1); update both occurrences so the
debug and non-debug branches use inner.result directly (leave the surrounding
template strings and status logic intact) — change the slices in the args
assignment within the isPromise* handling to use inner.result instead of
inner.result.slice(0, -1) so the generated calls to
__intrinsic__peekPromiseStatus receive the full expression.
In `@src/js/builtins.d.ts`:
- Around line 137-140: The declared return type of $peekPromiseSettledValue is
incorrect: it currently returns V unconditionally but the JSDoc says it may
return undefined for pending promises; update the declaration for function
$peekPromiseSettledValue<V>(promise: Promise<V>) to return V | undefined so
callers are forced to handle the pending case and the type matches the
documented behavior.
In `@src/js/builtins/BundlerPlugin.ts`:
- Around line 257-261: In BundlerPlugin's logic where deferred onStart()
promises are enqueued (the block checking $isPromise(ret) and
$peekPromiseStatus(ret)), mark the promise as handled before stashing it by
calling $pokePromiseAsHandled(ret) immediately prior to pushing into
self.promises; this change in the onStart/setup path (used by
loadAndResolvePluginsForServe()) prevents transient unhandled-rejection warnings
for rejected promises that are later awaited via Promise.all.
In `@src/jsc/bindings/bindings.cpp`:
- Around line 3801-3806: The code is manually setting JSC::JSPromise flags
(checking JSC::JSPromise::isFirstResolvingFunctionCalledFlag and OR-ing
JSC::JSPromise::isHandledFlag) which bypasses the proper bookkeeping; replace
the direct flag manipulation with a call to promise->markAsHandled() wherever
you currently set isHandledFlag (e.g., the branch that checks
isFirstResolvingFunctionCalledFlag around the rejected-promise handling and the
analogous site later in the file), so that markAsHandled() performs the
canonical handling transition and updates any related internal state.
In `@src/jsc/bindings/BunPlugin.cpp`:
- Around line 850-853: The code currently mutates a plugin-owned rejected
promise by calling promise->setFlags(...) to make it appear fulfilled; instead
remove that setFlags call and mark the rejection as handled by calling
promise->markAsHandled() (follow the pattern used in
ModuleLoader.cpp/ZigGlobalObject.cpp), then read result = promise->result() and
return JSValue::encode(result) without altering the promise flags or other bits
so you don't clear/shared state on the original JSPromise::Status::Rejected.
In `@src/jsc/bindings/ZigGlobalObject.cpp`:
- Around line 1738-1743: The function jsBunPeekPromiseStatus conflates "not a
promise" with the enum value 0 (pending) by returning jsNumber(0) when
peekPromiseArgument(callFrame) fails; change the guard in jsBunPeekPromiseStatus
so that when peekPromiseArgument returns null/invalid you return a sentinel
numeric value outside the promise-status enum range (e.g., -1 or a value > max
enum) instead of 0, so callers can distinguish "not a promise" from "pending" —
update the return expression and any callers that check for 0 to treat the
sentinel as the invalid-input case; reference jsBunPeekPromiseStatus and
peekPromiseArgument to locate the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 44479310-3a75-4020-8776-7776de340d02
📒 Files selected for processing (18)
scripts/build/deps/webkit.tssrc/codegen/create_hash_tablesrc/codegen/replacements.tssrc/js/builtins.d.tssrc/js/builtins/BunBuiltinNames.hsrc/js/builtins/BundlerPlugin.tssrc/js/builtins/CommonJS.tssrc/js/builtins/Peek.tssrc/js/builtins/ReadableStreamInternals.tssrc/js/builtins/StreamInternals.tssrc/js/builtins/WritableStreamInternals.tssrc/js/internal/util/inspect.jssrc/jsc/JSType.rssrc/jsc/JSType.zigsrc/jsc/bindings/BunPlugin.cppsrc/jsc/bindings/ZigGlobalObject.cppsrc/jsc/bindings/ZigGlobalObject.hsrc/jsc/bindings/bindings.cpp
…on builds The non-debug branch concatenated a stray open paren before the sliced inner.result, leaving the expanded expression with one unclosed group: (__intrinsic__peekPromiseStatus((promise) === 1). Use inner.result verbatim (it already includes the surrounding parens) so the expansion becomes (__intrinsic__peekPromiseStatus(promise) === 1).
…cord order The promise reaction rework allocation-frees the first then/await reaction by packing it into the JSPromise itself (InlineReactionKind::InternalMicrotask carries the await generator context in m_slot). getAwaitingGenerator only walked the heap JSPromiseReaction list, so async stack frames vanished for the common single-await case. Handle inline reactions before falling back to the heap list. Going through HashMap<String, String> for CookieMap's record initializer scrambles insertion order, and the order shifted when WTF moved its string hash to RapidHash. Build a Vector<Vector<String>> sequence instead and update the basic-iterator snapshot to the now-deterministic insertion order.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/build/deps/webkit.ts`:
- Line 6: The current WEBKIT_VERSION value ("autobuild-preview-pr-231-5568b798")
is too long and non-unique when prebuiltDestDir() truncates
cfg.webkitVersion.slice(0, 16), causing different autobuild tags to collide;
update either WEBKIT_VERSION or the prebuiltDestDir logic so the cache key
preserves enough entropy: change WEBKIT_VERSION to a shorter, stable identifier
(e.g., a unique build hash or numeric id) or modify prebuiltDestDir() to include
a longer slice or a hash of cfg.webkitVersion instead of slice(0,16) so
different autobuild tags (referencing WEBKIT_VERSION and
prebuiltDestDir()/cfg.webkitVersion.slice(0,16)) produce distinct prebuilt cache
directories.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0a90fbcf-d2e9-47b2-891d-e5e5a9ec038a
📒 Files selected for processing (4)
scripts/build/deps/webkit.tssrc/jsc/bindings/bindings.cppsrc/jsc/bindings/webcore/JSCookieMap.cpptest/js/bun/cookie/cookie-map.test.ts
oven-sh/WebKit#231 landed; switch from the preview-build tag to the release built from main.
…hrows The streaming hook used to return a freshly created promise. The host function caller (webAssemblyCompileStreamingFunc) propagated any pending exception into a rejected promise. After the upstream rework the caller allocates the outer promise itself, the hook returns void, and the new caller (the webAssemblyCompileStreaming internal microtask) does not catch exceptions. Any throw out of the hook left the outer promise permanently pending. Reject it before returning instead.
- JSType.zig: move the JSMicrotaskDispatcher doc comment back below the WebAssemblyStreamingContext entry so each entry has the right comment (matches the .rs mirror). - builtins.d.ts: $peekPromiseSettledValue documents that it returns undefined for a pending promise — make the type signature match. - bindings.cpp: use markAsHandled() instead of twiddling isHandledFlag directly in JSC__JSPromise__result and JSC__JSInternalPromise__setHandled. Same effect, but the intent reads more clearly. - BundlerPlugin.ts: mark deferred onStart() promises as handled when they are stashed on self.promises. The aggregate Promise.all handler is only attached later in loadAndResolvePluginsForServe(), and a synchronous rejection landing while the promise is just sitting in the array would trip the unhandled-rejection tracker.
Upgrades WebKit to `autobuild-preview-pr-231-2b257999` (commit `2b257999114b`) (oven-sh/WebKit#231 — ~755 upstream commits since `5488984d20e0`). ## Bun-side changes ### `JSPromise` rework (`3f9955f695b6`) `JSPromise` is no longer `JSInternalFieldObjectImpl<2>`. The `getPromiseInternalField`/`putPromiseInternalField` bytecode intrinsics and the `promiseField*`/`promiseState*` intrinsic constants were removed upstream. Added three C++ host functions exposed as private globals — `$peekPromiseStatus(p)` (`0`/`1`/`2` = pending/fulfilled/rejected), `$peekPromiseSettledValue(p)`, and `$pokePromiseAsHandled(p)` — and rewrote all 26 builtin call sites across `Peek.ts`, `CommonJS.ts`, `BundlerPlugin.ts`, `StreamInternals.ts`, `WritableStreamInternals.ts`, `ReadableStreamInternals.ts`, and `internal/util/inspect.js`. The `$isPromiseFulfilled`/`$isPromiseRejected`/`$isPromisePending` codegen macros now expand to `$peekPromiseStatus(...) === N`. C++ bindings (`bindings.cpp`, `BunPlugin.cpp`) that touched `JSPromise::Field`/`internalField()` switched to `flags()`/`setFlags()`/`setSlot()`/`payloadCell()`. ### RapidHash (`ee2220df2080`) WTF replaced WyHash/SuperFastHash with RapidHash. The static-property lookup tables (`*.lut.h`) embed the string hash, so the `create_hash_table` perl script must produce the same hash the runtime computes. Replaced `src/codegen/create_hash_table` with the upstream RapidHash version and re-grafted Bun's `ConstantInteger` extension. Without this every static property on `JSGlobalObject` (`Bun`, `fetch`, `process`, ...) is invisible. ### `JSType` enum (`24cf2e544f58`) `JSWebAssemblyStreamingContextType` was inserted at slot 27 — `src/jsc/JSType.rs`/`.zig` re-numbered. ### WebAssembly streaming hooks `compileStreaming`/`instantiateStreaming` global hook signatures changed: the `JSPromise*` is now passed in instead of returned. ### `JSModuleNamespaceObject`/`AbstractModuleRecord` `getModuleNamespace()` gains a `ModulePhase` argument (`import defer`); Bun's `shouldPreventExtensions` parameter is preserved as a trailing default. ## Known regression (pre-existing investigation) `Error inside minified file` snapshot tests in `inspect-error.test.js` show an extra `at require (native:50:24)` frame. The `require` builtin's `ImplementationVisibility::Private` is no longer hiding the frame after the upstream PCH/builtin refactor — needs follow-up. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
- wtf-bindings.cpp: assert -> ASSERT (same as #30992; #30705 dropped the transitive <cassert> include so debug builds fail to compile) - semver.test.ts: 30s timeout on the 1e5-iteration memory-churn test and the subprocess regression test; both exceed the 5s default under debug+ASAN on constrained runners
…#31010) A version range with >=256 `||`/whitespace-separated comparators panicked the **debug** build (`attempt to add with overflow`, exit 133) where released Bun fails gracefully. Debug-only: Cargo's dev profile enables `overflow-checks`; the release profile doesn't, so release wrapped (256 -> 0) exactly like Zig's ReleaseFast. `SemverQuery::parse` used `let mut count: u8 = 0;` with `count += 1;` — the 256th increment overflows. `count` is only ever compared `== 0`, never stored or cast, so a wider type is strictly more correct (it also avoids Zig's silent u8 wrap, which was a latent bug that merely didn't crash). Widen `count` to `u32`. Regression test (`Bun.semver.satisfies` with 300 `||` comparators, run in a subprocess since the failure was an uncatchable abort) passes on the debug build and matches released Bun. --- Also carried: - `wtf-bindings.cpp`: `assert` → `ASSERT` — identical to #30992; the WebKit upgrade in #30705 dropped the transitive `<cassert>` include, so debug builds currently fail to compile on `main`. Needed here so the regression test can run under `bun bd`. - 30s timeout on the pre-existing `failures does not cause weird memory issues` test (1e5 iterations) and the new subprocess test — both exceed the 5s default under debug+ASAN. ### Verification ``` $ bun bd test test/cli/install/semver.test.ts 23 pass 0 fail $ ./build/release/bun test test/cli/install/semver.test.ts 23 pass 0 fail ``` With the `u32` change reverted, the new test fails on debug with `panic: attempt to add with overflow (src/semver/SemverQuery.rs:981)` on stderr and exit 132. --------- Co-authored-by: robobun <117481402+robobun@users.noreply.github.com>
Upgrades WebKit to
autobuild-preview-pr-231-2b257999(commit2b257999114b) (oven-sh/WebKit#231 — ~755 upstream commits since5488984d20e0).Bun-side changes
JSPromiserework (3f9955f695b6)JSPromiseis no longerJSInternalFieldObjectImpl<2>. ThegetPromiseInternalField/putPromiseInternalFieldbytecode intrinsics and thepromiseField*/promiseState*intrinsic constants were removed upstream. Added three C++ host functions exposed as private globals —$peekPromiseStatus(p)(0/1/2= pending/fulfilled/rejected),$peekPromiseSettledValue(p), and$pokePromiseAsHandled(p)— and rewrote all 26 builtin call sites acrossPeek.ts,CommonJS.ts,BundlerPlugin.ts,StreamInternals.ts,WritableStreamInternals.ts,ReadableStreamInternals.ts, andinternal/util/inspect.js. The$isPromiseFulfilled/$isPromiseRejected/$isPromisePendingcodegen macros now expand to$peekPromiseStatus(...) === N.C++ bindings (
bindings.cpp,BunPlugin.cpp) that touchedJSPromise::Field/internalField()switched toflags()/setFlags()/setSlot()/payloadCell().RapidHash (
ee2220df2080)WTF replaced WyHash/SuperFastHash with RapidHash. The static-property lookup tables (
*.lut.h) embed the string hash, so thecreate_hash_tableperl script must produce the same hash the runtime computes. Replacedsrc/codegen/create_hash_tablewith the upstream RapidHash version and re-grafted Bun'sConstantIntegerextension. Without this every static property onJSGlobalObject(Bun,fetch,process, ...) is invisible.JSTypeenum (24cf2e544f58)JSWebAssemblyStreamingContextTypewas inserted at slot 27 —src/jsc/JSType.rs/.zigre-numbered.WebAssembly streaming hooks
compileStreaming/instantiateStreamingglobal hook signatures changed: theJSPromise*is now passed in instead of returned.JSModuleNamespaceObject/AbstractModuleRecordgetModuleNamespace()gains aModulePhaseargument (import defer); Bun'sshouldPreventExtensionsparameter is preserved as a trailing default.Known regression (pre-existing investigation)
Error inside minified filesnapshot tests ininspect-error.test.jsshow an extraat require (native:50:24)frame. Therequirebuiltin'sImplementationVisibility::Privateis no longer hiding the frame after the upstream PCH/builtin refactor — needs follow-up.