Repository navigation
Conversation
alii
changed the title
The changes improve state handling during client-side navigation by: - Using correct function updates with setAppState - Preserving abort controller on updates - Adding proper cache ID comparison - Moving router instance to constants
The improved types ensure only valid HTML attributes can be set for each element type, making the code more type-safe and self-documenting.
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (5)
⛔ Files ignored due to path filters (2)
You can disable this status message by setting the WalkthroughAdds a new standalone package Changes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 25
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/bake.zig (1)
562-575: Critical: ignoreDirs parsing uses wrong iterator and wrong length; fix off-by-many and OOB risks
- Uses
array.arrayIteratorinstead ofexts_js.arrayIterator.- Allocates
dirswithlen(routerTypes length) instead of the number of ignoreDirs entries.- Error message mentions “extensions”.
This can cause incorrect sizes and out-of-bounds writes.
- const ignore_dirs: []const []const u8 = if (try fsr_opts.get(global, "ignoreDirs")) |exts_js| exts: { - if (exts_js.isArray()) { - var it_2 = try array.arrayIterator(global); - var i_2: usize = 0; - const dirs = try arena.alloc([]const u8, len); - while (try it_2.next()) |array_item| : (i_2 += 1) { - dirs[i_2] = refs.track(try array_item.toSlice(global, arena)); - } - break :exts dirs; - } - - return global.throwInvalidArguments("'ignoreDirs' must be an array of strings or \"*\" for all extensions", .{}); - } else &.{ ".git", "node_modules" }; + const ignore_dirs: []const []const u8 = if (try fsr_opts.get(global, "ignoreDirs")) |exts_js| exts: { + if (exts_js.isArray()) { + var it_2 = try exts_js.arrayIterator(global); + var i_2: usize = 0; + const count = try exts_js.getLength(global); + const dirs = try arena.alloc([]const u8, count); + while (try it_2.next()) |array_item| : (i_2 += 1) { + dirs[i_2] = refs.track(try array_item.toSlice(global, arena)); + } + break :exts dirs; + } + + return global.throwInvalidArguments("'ignoreDirs' must be an array of strings", .{}); + } else &.{ ".git", "node_modules" };packages/bun-framework-react/server.tsx (5)
94-99: CSS metadata length is wrong (use bytes, not string length).str.length counts code units, not bytes. This truncates non‑ASCII and breaks CSS preloading.
Apply:
- const int = Buffer.allocUnsafe(4); - const str = meta.styles.join("\n"); - int.writeUInt32LE(str.length, 0); - rscPayload.write(int); - rscPayload.write(str); + const cssMeta = Buffer.from(meta.styles.join("\n"), "utf8"); + const int = Buffer.allocUnsafe(4); + int.writeUInt32LE(cssMeta.byteLength, 0); + rscPayload.write(int); + rscPayload.write(cssMeta);
172-176: Prerender header/value mismatch (writes count, not byte length).You’re writing the number of styles, then a string; reader expects a byte length header and raw bytes.
Apply:
- const int = new Uint32Array(1); - int[0] = meta.styles.length; - let rscChunks: Array<BlobPart> = [int.buffer as ArrayBuffer, meta.styles.join("\n")]; + const enc = new TextEncoder(); + const cssMeta = enc.encode(meta.styles.join("\n")); + const int = new Uint32Array(1); + int[0] = cssMeta.byteLength; + let rscChunks: Array<BlobPart> = [int.buffer, cssMeta];
117-124: Avoid overriding framework-set headers; normalize header merging.Spreading responseOptions after a headers literal can silently override Content‑Type. Normalize merge with Headers.
Apply:
- const responseOptions = als?.getStore()?.responseOptions || {}; - return new Response(rscPayload as any, { - status: 200, - headers: { "Content-Type": "text/x-component" }, - ...responseOptions, - }); + const opts = als?.getStore()?.responseOptions ?? { headers: {} }; + const { headers: extraHeaders, ...response_options } = opts; + const finalHeaders = new Headers(extraHeaders); + finalHeaders.set("Content-Type", "text/x-component"); + return new Response(rscPayload as any, { + status: 200, + headers: finalHeaders, + ...response_options, + });
128-139: Same header merge issue in streaming branch.Ensure Content‑Type isn’t lost when responseOptions includes headers.
Apply:
- const responseOptions = als?.getStore()?.responseOptions || {}; - if (als) { + const opts = als?.getStore()?.responseOptions ?? { headers: {} }; + if (als) { const state = als.getStore(); if (state) state.streamingStarted = true; } - // Stream the response as before - return new Response(renderToHtml(rscPayload, meta.modules, signal), { - headers: { - "Content-Type": "text/html; charset=utf8", - }, - ...responseOptions, - }); + const { headers: extraHeaders, ...response_options } = opts; + const finalHeaders = new Headers(extraHeaders); + finalHeaders.set("Content-Type", "text/html; charset=utf8"); + return new Response(renderToHtml(rscPayload, meta.modules, signal), { + headers: finalHeaders, + ...response_options, + });
145-158: Use Headers API to append multiple Set-Cookie headersA plain object can’t carry multiple “Set-Cookie” values. Construct a Headers instance, set Content-Type, then append each cookie from
request.cookies.toSetCookieHeaders():- const opts = als?.getStore()?.responseOptions ?? { headers: {} }; - const { headers, ...response_options } = opts; - - const cookies = meta.pageModule.mode === "ssr" ? { "Set-Cookie": request.cookies.toSetCookieHeaders() } : {}; - - return new Response(result, { - headers: { - "Content-Type": "text/html; charset=utf8", - // TODO: merge cookies and cookies inside of headers - ...cookies, - ...headers, - }, - ...response_options, - }); + const opts = als?.getStore()?.responseOptions ?? { headers: {} }; + const { headers: extraHeaders, ...response_options } = opts; + const finalHeaders = new Headers(extraHeaders); + finalHeaders.set("Content-Type", "text/html; charset=utf8"); + const cookieHeaders = meta.pageModule.mode === "ssr" + ? request.cookies.toSetCookieHeaders() + : []; + for (const c of cookieHeaders) finalHeaders.append("Set-Cookie", c); + return new Response(result, { + headers: finalHeaders, + ...response_options, + });packages/bun-framework-react/ssr.tsx (1)
75-85: Potential hang: stream.finished never resolves on error.onError closes the controller and aborts React, but doesn’t reject the RscInjectionStream’s finished promise. pull() returns stream.finished, so callers can hang.
Apply:
onError(error) { if (!signal.aborted) { // Abort the rendering and close the stream signal.aborted = error as Error; abort(); if (signal.abort) signal.abort(); if (stream) { - stream.controller.close(); + // Reject finished so pull() completes with error, then close + stream.reject(error); + stream.controller.close(); } } },
♻️ Duplicate comments (1)
packages/bun-framework-react/vendor/react-server-dom-bun/static.node.unbundled.js (1)
4-8: Same NODE_ENV fallback consideration applies.Like
static.node.js, this file treats any non-'production'value as development. The same optional validation suggestion applies here.
🧹 Nitpick comments (30)
src/bake/client/stack-trace.ts (1)
55-82: Consider reverting to inline object return for consistency and conciseness.The explicit return type annotation on line 55 and the two-step variable-then-return pattern (lines 74-81) add verbosity without clear benefit:
- TypeScript can infer the
Framereturn type from the object shape and interface- The intermediate variable doesn't improve readability for this simple transformation
- This creates inconsistency with
parseFFOrSafari(lines 85-116), which uses inline object returnsThe original pattern (inline object with
satisfies Frameif type assertion was needed) is more idiomatic for.map()callbacks in modern TypeScript.Consider this more concise approach:
- .map((line): Frame => { + .map((line) => { let sanitizedLine = line .replace(/^\s+/, "") .replace(/\(eval code/g, "(") .replace(/^.*?\s+/, ""); // capture and preserve the parenthesized location "(/foo/my bar.js:12:87)" in // case it has spaces in it, as the string is split on \s+ later on let loc = sanitizedLine.match(/ (\(.+\)$)/); // remove the parenthesized location from the line, if it was matched sanitizedLine = loc ? sanitizedLine.replace(loc[0], "") : sanitizedLine; // if a location was matched, pass it to extractLocation() otherwise pass all sanitizedLine // because this line doesn't have function name let locationParts = extractLocation(loc ? loc[1] : sanitizedLine); let functionName = (loc && sanitizedLine) || undefined; let fileName = ["eval", "<anonymous>"].indexOf(locationParts[0]) > -1 ? undefined : locationParts[0]; - const frame: Frame = { + return { fn: functionName || "unknown", file: fileName, line: 0 | locationParts[1], col: 0 | locationParts[2], }; - - return frame; });.vscode/launch.json (1)
60-64: Rename to match behavior and verify cwd for app buildsThis config is named “bun run [file]” but now runs “build --app” with cwd = ${fileDirname}. That’s confusing and may break workflows that expect to run the current file. Recommend:
- Rename to “bun build --app” (or add a new config for build and keep the old “run [file]”).
- Verify whether the app expects to resolve from workspace root; if so, set cwd = ${workspaceFolder}.
- Consider aligning the Windows config with this behavior for parity.
Example minimal fix:
- "name": "bun run [file]", + "name": "bun build --app", - "args": ["build", "--app"], + "args": ["build", "--app"], - "cwd": "${fileDirname}", + "cwd": "${workspaceFolder}",Based on learnings
src/bake/client/overlay.ts (2)
82-98: Consider usingObject.keys()orhasOwnPropertywithfor...in.The
for...inloop iterates over all enumerable properties, including inherited ones. While this is likely safe given the expected plain object usage, usingObject.keys(props)would be more explicit and avoid potential issues if props ever has a prototype chain.Optional refactor:
if (props) { - for (const key in props) { + for (const key of Object.keys(props)) { const value = props[key]; if (value === undefined) continue; node.setAttribute(key, value); } }
100-111: Consider usingObject.keys()for consistency withelem.The same
for...inconsideration applies here. UsingObject.keys(props)would avoid iterating over inherited properties.Optional refactor:
if (props) { - for (const key in props) { + for (const key of Object.keys(props)) { const value = props[key]; if (value === undefined) continue; node.setAttribute(key, value); } }src/ast/visitExpr.zig (1)
36-48: Clarify the intent of the commented-out hook list.The commented-out list of well-known client-only React hooks (useState, useEffect, etc.) suggests incomplete implementation or future use. Since this is dead code with no functional impact, consider:
- If this list is intended for future use, add a TODO comment explaining when/how it will be used.
- If this list is no longer needed, remove it to avoid clutter.
Given that this PR is marked [WIP] and the build is failing, could you clarify whether this commented-out code should remain? If it's intended for future use, adding a brief TODO comment would improve clarity.
src/bake/BakeGlobalObject.cpp (1)
143-145: Consider validating URL before constructing SourceOrigin.The code asserts that the URL is valid (line 144), but this only runs in debug builds. If
moduleKeyis malformed (e.g., contains invalid characters or spaces afterbake:/),WTF::URLmight be invalid in release builds, potentially causing issues in source mapping or debugging.Consider validating the URL and handling the error case:
WTF::URL url = WTF::URL(moduleKey); - ASSERT(url.isValid()); + if (!url.isValid()) [[unlikely]] { + return rejectedInternalPromise(globalObject, + createTypeError(globalObject, makeString("Invalid bake:/ URL: "_s, moduleKey))); + } JSC::SourceOrigin origin = JSC::SourceOrigin(WTFMove(url));packages/bun-framework-react/tsconfig.json (2)
3-3: Add DOM.Iterable to libImproves typings for iterable DOM collections (e.g., URLSearchParams.entries(), NodeListOf iteration).
- "lib": ["ESNext", "DOM"], + "lib": ["ESNext", "DOM", "DOM.Iterable"],
2-23: Include Bun types in tsconfig
In packages/bun-framework-react/tsconfig.json, add undercompilerOptions:"compilerOptions": { + "types": ["bun-types"],This ensures Bun’s global APIs are recognized.
packages/bun-framework-react/vendor/react-server-dom-bun/static.node.js (1)
4-8: Consider explicit NODE_ENV fallback or validation.The current logic treats any non-
'production'value as development. While this is a common pattern, it silently loads the development bundle for typos like'productoin'or unexpected values like'staging'.For clarity, consider explicitly checking for known values:
var s; -if (process.env.NODE_ENV === 'production') { +if (process.env.NODE_ENV === 'production') { s = require('./cjs/react-server-dom-bun-server.node.production.js'); +} else if (process.env.NODE_ENV === 'development' || !process.env.NODE_ENV) { + s = require('./cjs/react-server-dom-bun-server.node.development.js'); } else { - s = require('./cjs/react-server-dom-bun-server.node.development.js'); + throw new Error(`Unexpected NODE_ENV: ${process.env.NODE_ENV}`); }Alternatively, keep the current behavior if the fallback-to-dev pattern is intentional for this vendored package.
packages/bun-framework-react/vendor/react-server-dom-bun/static.node.unbundled.js (1)
1-12: Consider extracting the environment-detection pattern.Both
static.node.jsandstatic.node.unbundled.jsduplicate the NODE_ENV detection logic (lines 1-8). While the duplication is minimal, extracting a shared helper could improve maintainability.Example helper module (
env-loader.js):'use strict'; exports.loadEnvSpecific = function(prodPath, devPath) { if (process.env.NODE_ENV === 'production') { return require(prodPath); } else { return require(devPath); } };Then simplify both files:
'use strict'; var loadEnvSpecific = require('./env-loader').loadEnvSpecific; var s = loadEnvSpecific( './cjs/react-server-dom-bun-server.node.unbundled.production.js', './cjs/react-server-dom-bun-server.node.unbundled.development.js' ); if (s.unstable_prerenderToNodeStream) { exports.unstable_prerenderToNodeStream = s.unstable_prerenderToNodeStream; }This is a minor refactor and can be deferred if preferred for this vendored code.
packages/bun-framework-react/vendor/react-server-dom-bun/server.js (1)
3-6: Consider adjusting error message for Bun context.The error message references Node.js specifically (
--conditions react-serverflag), but this file is part ofbun-framework-react. Bun users may find the Node.js-specific instruction confusing. Consider whether the message should mention Bun's equivalent mechanism or provide Bun-specific guidance.packages/bun-framework-react/vendor/react-server-dom-bun/static.js (1)
1-6: Clarify purpose of duplicate environment guard and adjust messaging.This file is identical to
server.js. If both serve as entry points for different module resolution conditions, consider:
- Deduplication: Extract the guard logic to a shared module that both can require.
- Bun-specific messaging: The error references Node.js
--conditions react-server, but this is in a Bun package. Adjust the message to provide Bun-specific guidance or clarify cross-runtime usage.Verify the distinct purposes of
server.jsandstatic.js:#!/bin/bash # Description: Check if server.js and static.js are referenced differently # Look for references to these specific entry points rg -n "static\.js|server\.js" packages/bun-framework-react/vendor/react-server-dom-bun/package.json rg -n "(require|import).*['\"].*/(static|server)\.js" packages/bun-framework-react/packages/bun-framework-react/vendor/react-server-dom-bun/client.js (1)
3-3: Forwarding to browser build may mis-route in Node if consumers bypass "exports".If a consumer deep-imports "…/client.js" ignoring the exports map, they'll always get the browser build. Either remove this file (prefer relying on package.json exports), or make it conditional to avoid surprises.
Example conditional shim:
-'use strict'; - -module.exports = require('./client.browser'); +'use strict'; + +// Prefer package.json "exports" map. This is a safety guard for tools that bypass it. +const isNode = typeof process !== 'undefined' && !!(process.versions && process.versions.node); +module.exports = isNode ? require('./client.node') : require('./client.browser');packages/bun-framework-react/src/client/store.ts (1)
22-33: Avoid relying on this binding in write; and return void unsubscribe.Safer to use the closed-over value instead of this.read(). Also adapt unsubscribe to return void.
- write(next) { - const current = this.read(); + write(next) { + const current = value; const resolved = next instanceof Function ? next(current) : next; if (Object.is(current, resolved)) return; value = resolved; notify(subscribers); }, @@ - subscribe(callback) { + subscribe(callback) { subscribers.add(callback); - return () => subscribers.delete(callback); + return () => { + subscribers.delete(callback); + }; },packages/bun-framework-react/vendor/react-server-dom-bun/package.json (1)
81-83: Node engines field seems too permissive.RSC stacks generally require modern runtimes (≥18). Consider tightening engines to reflect reality and avoid unsupported installs.
packages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-client.browser.production.js (1)
659-660: Treat vendor bundle as lint/format ignore; do not “fix” upstream semantics.Biome flags (thenable prototype, switch fallthrough, declarations in cases, shadowing “constructor”) are expected in React’s compiled runtime. Please exclude
packages/bun-framework-react/vendor/**from Biome and Prettier instead of modifying vendored code.Suggested config updates:
- .biome.json (or biome.json):
{ "files": { "ignore": ["packages/bun-framework-react/vendor/**"] } }
- .prettierignore:
packages/bun-framework-react/vendor/Also applies to: 746-759, 920-925, 1661-1661
packages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-client.node.production.js (1)
659-660: Exclude vendor from Biome/Prettier; compiled patterns are intentional.Flags for prototype
then, fallthrough, case-scoped declarations, and restricted-name shadowing are intentional in upstream React builds. Ignorepackages/bun-framework-react/vendor/**in lint/format to prevent noisy failures.Also applies to: 746-759, 920-925, 1661-1661, 1785-1785
packages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-server.node.unbundled.production.js (1)
467-467: Silence lints for vendored server runtime; do not modify upstream.Multiple Biome warnings (switch declarations, restricted-name shadowing, prototype
then, use-before-declaration) stem from React’s compiled server runtime. Excludepackages/bun-framework-react/vendor/**from Biome/Prettier rather than patching this file.Also applies to: 532-532, 919-919, 1147-1148, 1821-1822, 2085-2085, 2118-2118, 2342-2343, 402-405
packages/bun-framework-react/src/client/root.tsx (1)
16-26: Add effect deps and cancel the scheduled animation frameRunning this layout effect on every render without cleanup can queue redundant RAFs. Scope it to relevant changes and cancel on re-render/unmount.
- useLayoutEffect(() => { + useLayoutEffect(() => { if (app.abortOnRender) { try { app.abortOnRender.abort(); } catch {} } - requestAnimationFrame(() => { - router.css.disableUnusedCssFilesIfNeeded(); - }); - }); + const id = requestAnimationFrame(() => { + router.css.disableUnusedCssFilesIfNeeded(); + }); + return () => cancelAnimationFrame(id); + }, [app.abortOnRender]);packages/bun-framework-react/client.tsx (1)
39-48: Harden view-transition stylesheet injectionGuard for adoptedStyleSheets/CSSStyleSheet support to avoid runtime errors on partially supported browsers.
- if (document.startViewTransition !== undefined) { + if ( + document.startViewTransition !== undefined && + "adoptedStyleSheets" in document && + typeof CSSStyleSheet !== "undefined" + ) { const sheet = new CSSStyleSheet(); document.adoptedStyleSheets.push(sheet); sheet.replaceSync(":where(*)::view-transition-group(root){animation:none}"); }packages/bun-framework-react/vendor/react-server-dom-bun/client.node.unbundled.d.ts (2)
9-12: Narrow crossOrigin type for correctnessLimit to valid values to catch mistakes at compile time.
export interface ModuleLoading { prefix: string; - crossOrigin?: string; + crossOrigin?: "anonymous" | "use-credentials"; }
14-18: Avoid loose any in public typesIf possible, prefer unknown or specific function types to improve type safety.
export interface Options { - encodeFormAction?: any; - findSourceMapURL?: any; + encodeFormAction?: (x: unknown) => unknown; + findSourceMapURL?: (source: string) => string | null | undefined; environmentName?: string; }packages/bun-framework-react/vendor/react-server-dom-bun/server.node.unbundled.d.ts (2)
4-8: Avoid generic shadowing inside PipeableStreamRename the method generic to prevent shadowing the interface’s T.
export interface PipeableStream<T> { /** Returns the input, which should match the Node.js writable interface */ - pipe: <T extends NodeJS.WritableStream>(destination: T) => T; + pipe: <W extends NodeJS.WritableStream>(destination: W) => W; abort: () => void; }
10-14: Consider removing unused generic parameter TIf T is not used elsewhere in PipeableStream, the generic on renderToPipeableStream adds noise.
-export function renderToPipeableStream<T = any>( +export function renderToPipeableStream( model: ReactElement, webpackMap: ServerManifest, options?: RenderToPipeableStreamOptions, -): PipeableStream<T>; +): PipeableStream<any>;packages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-client.node.unbundled.production.js (1)
1-2125: Do not lint vendor bundle; exclude from Biome/ESLint.Static-analysis flags (fallthrough, noSwitchDeclarations, noThenProperty, shadowing) stem from compiled React code. Exclude packages/bun-framework-react/vendor/** from linters to avoid noise.
Add ignore config (e.g., .biome.json, .eslintignore) to skip vendor/.
packages/bun-framework-react/ssr.tsx (3)
51-61: Duplicate abort check (remove one and avoid passing reason to close).Two identical early-abort blocks exist. Keep a single check and call controller.close() without an argument to avoid relying on nonstandard signature.
Apply:
- // If the signal is already aborted, we should not proceed - if (signal.aborted) { - controller.close(signal.aborted); - return Promise.reject(signal.aborted); - } - - // If the signal is already aborted, we should not proceed - if (signal.aborted) { - controller.close(signal.aborted); - return Promise.reject(signal.aborted); - } + // If the signal is already aborted, we should not proceed + if (signal.aborted) { + controller.close(); + return Promise.reject(signal.aborted); + }If Bun’s Direct controller supports a reason, reply and I can adjust accordingly.
318-321: Use fatal UTF‑8 decoding in static path for consistency.StaticRscInjectionStream uses TextDecoder without fatal; dynamic path uses fatal and falls back to base64 on invalid UTF‑8. Make static consistent.
Apply:
- writeManyFlightScriptData(this.rscPayloadChunks, new TextDecoder("utf-8"), { write: str => (string += str) }); + writeManyFlightScriptData( + this.rscPayloadChunks, + new TextDecoder("utf-8", { fatal: true }), + { write: str => (string += str) } + );
185-188: shellReady flag is written but never used.onShellReady()/shellReady don’t affect injection logic. Either wire it into write/drain decisions or remove.
Would you like me to gate RSC injection until shellReady to reduce early interleaving?
packages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-server.browser.production.js (1)
11-20: Vendor file: exclude from lint/format and verify Buffer polyfill in browser contexts.Static analysis flags (switch declarations, shadowing, .then on objects) stem from upstream minified code. Don’t refactor vendor sources; instead, disable linters for this file and ensure your build treats it as vendored.
- Add a header disable comment for your linters (Biome/ESLint/Prettier), e.g.:
/* biome-ignore file */- Confirm Buffer.byteLength is available wherever this module executes. If it can run in “browser” graphs, ensure Bun’s Buffer polyfill is injected.
I can add a repo-safe script to check which graphs include this file and whether Buffer polyfills are emitted where needed.
packages/bun-types/app.d.ts (1)
662-680: Make importSource optional (aligns with docs and usage).Avoid requiring the property with undefined; mark it optional.
Apply:
- interface ReactFastRefreshOptions { + interface ReactFastRefreshOptions { /** * ... */ - importSource: string | undefined; + importSource?: string; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📥 Commits
Reviewing files that changed from the base of the PR and between c8cb771 and d7217f26d52f44f9d10dc7b57d4aebdc26afa2b1.
⛔ Files ignored due to path filters (4)
bun.lockis excluded by!**/*.lockpackages/bun-framework-react/bun.lockis excluded by!**/*.locktest/bake/fixtures/react-spa-simple/bun.lockis excluded by!**/*.locktest/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (63)
.gitattributes(1 hunks).vscode/launch.json(1 hunks)cmake/Sources.json(1 hunks)package.json(1 hunks)packages/bun-framework-react/.gitignore(1 hunks)packages/bun-framework-react/README.md(1 hunks)packages/bun-framework-react/client.tsx(1 hunks)packages/bun-framework-react/index.ts(1 hunks)packages/bun-framework-react/package.json(1 hunks)packages/bun-framework-react/server.tsx(4 hunks)packages/bun-framework-react/src/client/app.ts(1 hunks)packages/bun-framework-react/src/client/constants.ts(1 hunks)packages/bun-framework-react/src/client/css.ts(1 hunks)packages/bun-framework-react/src/client/lib/util.ts(1 hunks)packages/bun-framework-react/src/client/root.tsx(1 hunks)packages/bun-framework-react/src/client/router.ts(1 hunks)packages/bun-framework-react/src/client/store.ts(1 hunks)packages/bun-framework-react/src/components/link.tsx(1 hunks)packages/bun-framework-react/ssr.tsx(14 hunks)packages/bun-framework-react/tsconfig.json(1 hunks)packages/bun-framework-react/vendor/.prettierignore(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/LICENSE(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/README.md(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-client.browser.production.js(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-client.node.production.js(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-client.node.unbundled.production.js(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-server.browser.production.js(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-server.node.production.js(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-server.node.unbundled.production.js(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/client.browser.d.ts(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/client.browser.js(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/client.js(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/client.node.js(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/client.node.unbundled.d.ts(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/client.node.unbundled.js(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/esm/package.json(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/index.js(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/package.json(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/plugin.js(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/server.browser.js(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/server.js(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/server.node.js(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/server.node.unbundled.d.ts(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/server.node.unbundled.js(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/static.browser.js(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/static.js(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/static.node.js(1 hunks)packages/bun-framework-react/vendor/react-server-dom-bun/static.node.unbundled.js(1 hunks)packages/bun-types/app.d.ts(1 hunks)packages/bun-types/index.d.ts(1 hunks)packages/bun-vscode/example/package.json(1 hunks)src/ast/visitExpr.zig(2 hunks)src/bake.zig(8 hunks)src/bake/BakeGlobalObject.cpp(2 hunks)src/bake/DevServer.zig(0 hunks)src/bake/bake.d.ts(0 hunks)src/bake/bake.private.d.ts(3 hunks)src/bake/bun-framework-react/client.tsx(0 hunks)src/bake/bun-framework-react/index.ts(0 hunks)src/bake/client/data-view.ts(1 hunks)src/bake/client/overlay.ts(6 hunks)src/bake/client/stack-trace.ts(2 hunks)src/bake/client/websocket.ts(1 hunks)
💤 Files with no reviewable changes (4)
- src/bake/bun-framework-react/index.ts
- src/bake/DevServer.zig
- src/bake/bake.d.ts
- src/bake/bun-framework-react/client.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Format JavaScript/TypeScript files with Prettier (bun run prettier)
Files:
packages/bun-framework-react/src/client/constants.tspackages/bun-framework-react/src/client/root.tsxpackages/bun-framework-react/vendor/react-server-dom-bun/client.browser.d.tspackages/bun-framework-react/vendor/react-server-dom-bun/client.node.unbundled.jspackages/bun-framework-react/vendor/react-server-dom-bun/static.node.unbundled.jspackages/bun-types/index.d.tspackages/bun-framework-react/src/client/css.tspackages/bun-framework-react/server.tsxpackages/bun-framework-react/client.tsxsrc/bake/client/websocket.tspackages/bun-framework-react/src/client/router.tssrc/bake/client/data-view.tspackages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-client.node.unbundled.production.jspackages/bun-framework-react/vendor/react-server-dom-bun/server.jspackages/bun-framework-react/src/components/link.tsxsrc/bake/client/overlay.tspackages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-client.browser.production.jspackages/bun-framework-react/vendor/react-server-dom-bun/static.node.jspackages/bun-framework-react/vendor/react-server-dom-bun/index.jssrc/bake/client/stack-trace.tspackages/bun-framework-react/src/client/app.tspackages/bun-framework-react/vendor/react-server-dom-bun/server.node.jspackages/bun-framework-react/vendor/react-server-dom-bun/client.browser.jspackages/bun-framework-react/src/client/lib/util.tspackages/bun-framework-react/vendor/react-server-dom-bun/client.node.jspackages/bun-framework-react/index.tspackages/bun-framework-react/vendor/react-server-dom-bun/plugin.jspackages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-client.node.production.jspackages/bun-framework-react/vendor/react-server-dom-bun/server.node.unbundled.d.tspackages/bun-framework-react/ssr.tsxpackages/bun-framework-react/vendor/react-server-dom-bun/server.node.unbundled.jspackages/bun-framework-react/src/client/store.tspackages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-server.node.unbundled.production.jspackages/bun-framework-react/vendor/react-server-dom-bun/server.browser.jspackages/bun-framework-react/vendor/react-server-dom-bun/static.jspackages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-server.node.production.jspackages/bun-framework-react/vendor/react-server-dom-bun/client.node.unbundled.d.tspackages/bun-framework-react/vendor/react-server-dom-bun/client.jspackages/bun-framework-react/vendor/react-server-dom-bun/static.browser.jssrc/bake/bake.private.d.tspackages/bun-types/app.d.tspackages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-server.browser.production.js
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Implement debug logs in Zig using
const log = bun.Output.scoped(.${SCOPE}, false);and invokinglog("...", .{})
Files:
src/ast/visitExpr.zigsrc/bake.zig
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
**/*.zig: Format Zig files with zig-format (bun run zig-format)
In Zig, manage memory carefully with allocators and use defer for cleanup
Files:
src/ast/visitExpr.zigsrc/bake.zig
**/*.{cpp,h}
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.{cpp,h}: When exposing a JS class with public Constructor and Prototype, define three C++ types: class Foo : public JSC::DestructibleObject (if it has C++ fields), class FooPrototype : public JSC::JSNonFinalObject, and class FooConstructor : public JSC::InternalFunction
If the class has C++ data members, inherit from JSC::DestructibleObject and provide proper destruction; if it has no C++ fields (only JS properties), avoid a class and use JSC::constructEmptyObject(vm, structure) with putDirectOffset
Prefer placing the subspaceFor implementation in the .cpp file rather than the header when possible
Files:
src/bake/BakeGlobalObject.cpp
**/*.cpp
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.cpp: Include "root.h" at the top of C++ binding files to satisfy lints
Define prototype properties using a const HashTableValue array and declare accessors/functions with JSC_DECLARE_* macros
Prototype classes should subclass JSC::JSNonFinalObject, provide create/createStructure, DECLARE_INFO, finishCreation that reifies static properties, and set mayBePrototype on the Structure
Custom getters should use JSC_DEFINE_CUSTOM_GETTER, jsDynamicCast to validate this, and throwThisTypeError on mismatch
Custom setters should use JSC_DEFINE_CUSTOM_SETTER, validate this via jsDynamicCast, and store via WriteBarrier/set semantics
Prototype functions should use JSC_DEFINE_HOST_FUNCTION, validate this with jsDynamicCast, and return encoded JSValue
Constructors should subclass JSC::InternalFunction, return internalFunctionSpace in subspaceFor, set the prototype property as non-configurable/non-writable, and provide create/createStructure
Provide a setup function that builds the Prototype, Constructor, and Structure, and assigns them to the LazyClassStructure initializer
Use the cached Structure via globalObject->m_.get(globalObject) when constructing instances
Expose constructors to Zig via an extern "C" function that returns the constructor from the LazyClassStructure
Provide an extern "C" Bun____toJS function that creates an instance using the cached Structure and returns an EncodedJSValue
Files:
src/bake/BakeGlobalObject.cpp
**/*.{cpp,cc,cxx,h,hpp,hxx}
📄 CodeRabbit inference engine (CLAUDE.md)
Format C/C++ sources/headers with clang-format (bun run clang-format)
Files:
src/bake/BakeGlobalObject.cpp
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/react-spa.test.ts : react-spa.test.ts should contain React SPA, react-refresh, and basic server component transform tests
📚 Learning: 2025-08-30T00:07:54.573Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/building-bun.mdc:0-0
Timestamp: 2025-08-30T00:07:54.573Z
Learning: To run a file, use `bun bd <file> <...args>`; never use `bun <file>` directly
Applied to files:
.vscode/launch.json
📚 Learning: 2025-09-07T05:41:52.563Z
Learnt from: CR
PR: oven-sh/bun#0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-09-07T05:41:52.563Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{js,ts} : Author modules as CommonJS-style with `require(...)` and export via `export default {}` (no ESM `import`/named exports)
Applied to files:
packages/bun-framework-react/vendor/react-server-dom-bun/client.node.unbundled.jspackages/bun-framework-react/vendor/react-server-dom-bun/client.browser.jspackages/bun-framework-react/vendor/react-server-dom-bun/plugin.js
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/css.test.ts : css.test.ts should contain CSS bundling tests in dev mode
Applied to files:
packages/bun-framework-react/src/client/css.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/react-spa.test.ts : react-spa.test.ts should contain React SPA, react-refresh, and basic server component transform tests
Applied to files:
packages/bun-framework-react/package.json
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/node/test/** : Vendored Node.js tests under test/js/node/test/ are exceptions and do not follow Bun’s test style
Applied to files:
.gitattributes
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests
Applied to files:
src/bake/bake.private.d.tspackages/bun-types/app.d.ts
🧬 Code graph analysis (13)
packages/bun-framework-react/src/client/constants.ts (1)
packages/bun-framework-react/src/client/router.ts (1)
Router(11-215)
packages/bun-framework-react/src/client/root.tsx (3)
packages/bun-framework-react/src/client/app.ts (1)
useAppState(86-88)packages/bun-framework-react/src/client/constants.ts (1)
router(3-3)packages/bun-framework-react/src/client/lib/util.ts (1)
isThenable(1-3)
packages/bun-framework-react/client.tsx (4)
packages/bun-framework-react/src/client/root.tsx (1)
Root(11-29)packages/bun-framework-react/src/client/app.ts (1)
initialRscPayloadThen(94-96)packages/bun-framework-react/src/client/constants.ts (1)
router(3-3)src/bake/hmr-module.ts (1)
onServerSideReload(32-32)
packages/bun-framework-react/src/client/router.ts (2)
packages/bun-framework-react/src/client/app.ts (4)
NonNullishReactNode(5-5)AppState(23-33)setAppState(82-84)getAppState(90-92)packages/bun-framework-react/src/client/css.ts (1)
BakeCSSManager(1-355)
packages/bun-framework-react/src/components/link.tsx (1)
packages/bun-framework-react/src/client/constants.ts (1)
router(3-3)
packages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-client.browser.production.js (1)
packages/bun-framework-react/ssr.tsx (2)
constructor(163-183)constructor(289-297)
packages/bun-framework-react/src/client/app.ts (1)
packages/bun-framework-react/src/client/store.ts (3)
Store(3-7)store(13-35)useStore(37-39)
packages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-client.node.production.js (1)
packages/bun-framework-react/ssr.tsx (2)
constructor(163-183)constructor(289-297)
packages/bun-framework-react/ssr.tsx (3)
packages/bun-framework-react/server.tsx (1)
MiniAbortSignal(238-242)packages/bun-framework-react/src/client/root.tsx (1)
Root(11-29)packages/bun-framework-react/vendor/react-server-dom-bun/server.node.unbundled.d.ts (1)
RenderToPipeableStreamOptions(16-23)
packages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-server.node.unbundled.production.js (2)
src/node-fallbacks/buffer.js (1)
Buffer(136-145)packages/bun-framework-react/ssr.tsx (2)
constructor(163-183)constructor(289-297)
packages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-server.node.production.js (1)
packages/bun-framework-react/ssr.tsx (2)
constructor(163-183)constructor(289-297)
packages/bun-types/app.d.ts (2)
src/bake/hmr-runtime-server.ts (1)
RequestContext(17-17)src/bake/hmr-module.ts (3)
serverManifest(28-28)ssrManifest(30-30)onServerSideReload(32-32)
packages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-server.browser.production.js (2)
src/node-fallbacks/buffer.js (1)
Buffer(136-145)packages/bun-framework-react/ssr.tsx (2)
constructor(163-183)constructor(289-297)
🪛 Biome (2.1.2)
packages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-client.node.unbundled.production.js
[error] 250-250: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 659-660: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 746-759: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 920-925: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 921-921: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1661-1661: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
[error] 1785-1785: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
packages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-client.browser.production.js
[error] 250-250: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 659-660: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 746-759: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 920-925: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 921-921: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1661-1661: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
packages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-client.node.production.js
[error] 250-250: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 659-660: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 746-759: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 920-925: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 921-921: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1661-1661: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
[error] 1785-1785: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
packages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-server.node.unbundled.production.js
[error] 467-467: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 532-532: Do not shadow the global "hasOwnProperty" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
[error] 919-919: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1147-1148: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1821-1822: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 2085-2085: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2118-2118: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
[error] 2342-2343: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 402-402: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 405-405: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
packages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-server.node.production.js
[error] 467-467: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 532-532: Do not shadow the global "hasOwnProperty" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
[error] 919-919: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1147-1148: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1821-1822: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 2085-2085: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2118-2118: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
[error] 2342-2343: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 402-402: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 405-405: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
packages/bun-framework-react/vendor/react-server-dom-bun/cjs/react-server-dom-bun-server.browser.production.js
[error] 464-464: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 529-529: Do not shadow the global "hasOwnProperty" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
[error] 916-916: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1144-1145: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1818-1819: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 2082-2082: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2115-2115: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
[error] 2339-2340: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 399-399: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 402-402: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
🪛 markdownlint-cli2 (0.18.1)
packages/bun-framework-react/README.md
1-1: Images should have alternate text (alt text)
(MD045, no-alt-text)
| window.addEventListener("popstate", async event => { | ||
| const state = typeof event.state === "number" ? event.state : undefined; | ||
| await router.navigate(location.href, state); | ||
| }); |
There was a problem hiding this comment.
Avoid unhandled rejections in popstate handler
Event listeners ignore returned Promises. Catch navigation errors to prevent "Unhandled promise rejection" noise.
-window.addEventListener("popstate", async event => {
- const state = typeof event.state === "number" ? event.state : undefined;
- await router.navigate(location.href, state);
-});
+window.addEventListener("popstate", event => {
+ const state = typeof event.state === "number" ? event.state : undefined;
+ void router.navigate(location.href, state).catch(console.error);
+});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| window.addEventListener("popstate", async event => { | |
| const state = typeof event.state === "number" ? event.state : undefined; | |
| await router.navigate(location.href, state); | |
| }); | |
| window.addEventListener("popstate", event => { | |
| const state = typeof event.state === "number" ? event.state : undefined; | |
| void router.navigate(location.href, state).catch(console.error); | |
| }); |
🤖 Prompt for AI Agents
In packages/bun-framework-react/client.tsx around lines 51 to 54, the popstate
event handler awaits router.navigate but does not catch rejections, which can
produce unhandled promise rejection noise; wrap the navigation call in a
try/catch (or append .catch(...)) and log or handle errors (include context like
the location.href and state) so any navigation failure is captured and does not
surface as an unhandled rejection.
| @@ -0,0 +1,10 @@ | |||
| <img src="https://bun.com/logo.png" height="36" /> | |||
There was a problem hiding this comment.
Add alt text to the image for accessibility.
The image is missing alternative text, which is important for accessibility and SEO.
Apply this diff to add descriptive alt text:
-<img src="https://bun.com/logo.png" height="36" />
+<img src="https://bun.com/logo.png" alt="Bun Logo" height="36" />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <img src="https://bun.com/logo.png" height="36" /> | |
| <img src="https://bun.com/logo.png" alt="Bun Logo" height="36" /> |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
1-1: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
In packages/bun-framework-react/README.md around lines 1 to 1, the <img> tag
lacks an alt attribute which harms accessibility and SEO; update the tag to
include a descriptive alt text such as alt="Bun logo" or alt="Bun JavaScript
runtime logo" (choose the wording that best fits the repo) so screen readers and
search engines can interpret the image; apply the change by adding the alt
attribute to the existing img element.
| An implementation of the Bun Rendering API for React, with RSC (React Server Components) | ||
|
|
||
| 1. `bun add bun-framework-react --dev` | ||
| 2. Make a `pages/index.tsx` file, with a page component defualt export |
There was a problem hiding this comment.
Fix typo in documentation.
Line 8 contains a typo: "defualt" should be "default".
Apply this diff to correct the typo:
-2. Make a `pages/index.tsx` file, with a page component defualt export
+2. Make a `pages/index.tsx` file, with a page component default export📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 2. Make a `pages/index.tsx` file, with a page component defualt export | |
| 2. Make a `pages/index.tsx` file, with a page component default export |
🤖 Prompt for AI Agents
In packages/bun-framework-react/README.md around line 8, the word "defualt" is
misspelled; change it to "default" so the sentence reads "...with a page
component default export". Update the typo and save the README.
| const link = document.createElement("link"); | ||
| let entry: { promise: Promise<void> | null; link: HTMLLinkElement }; | ||
|
|
||
| const promise = new Promise<void>((resolve, reject) => { | ||
| link.rel = "stylesheet"; | ||
| link.onload = resolve.bind(null, undefined); | ||
| link.onerror = reject; | ||
| link.href = href; | ||
| document.head.appendChild(link); | ||
| }).finally(() => { | ||
| entry.promise = null; | ||
| }); | ||
|
|
||
| entry = { promise, link }; | ||
| this.cssFiles.set(href, entry); | ||
| wait.push(promise); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fix TDZ/closure hazard when setting entry.promise and link creation.
entry is referenced inside finally before assignment. It’s safe in practice but brittle. Initialize entry first to avoid TDZ-like hazards.
- const link = document.createElement("link");
- let entry: { promise: Promise<void> | null; link: HTMLLinkElement };
-
- const promise = new Promise<void>((resolve, reject) => {
- link.rel = "stylesheet";
- link.onload = resolve.bind(null, undefined);
- link.onerror = reject;
- link.href = href;
- document.head.appendChild(link);
- }).finally(() => {
- entry.promise = null;
- });
-
- entry = { promise, link };
- this.cssFiles.set(href, entry);
- wait.push(promise);
+ const link = document.createElement("link");
+ const entry: { promise: Promise<void> | null; link: HTMLLinkElement } = { promise: null, link };
+ this.cssFiles.set(href, entry);
+
+ entry.promise = new Promise<void>((resolve, reject) => {
+ link.rel = "stylesheet";
+ link.onload = () => resolve();
+ link.onerror = reject;
+ link.href = href;
+ document.head.appendChild(link);
+ }).finally(() => {
+ entry.promise = null;
+ });
+
+ wait.push(entry.promise);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const link = document.createElement("link"); | |
| let entry: { promise: Promise<void> | null; link: HTMLLinkElement }; | |
| const promise = new Promise<void>((resolve, reject) => { | |
| link.rel = "stylesheet"; | |
| link.onload = resolve.bind(null, undefined); | |
| link.onerror = reject; | |
| link.href = href; | |
| document.head.appendChild(link); | |
| }).finally(() => { | |
| entry.promise = null; | |
| }); | |
| entry = { promise, link }; | |
| this.cssFiles.set(href, entry); | |
| wait.push(promise); | |
| } | |
| const link = document.createElement("link"); | |
| const entry: { promise: Promise<void> | null; link: HTMLLinkElement } = { promise: null, link }; | |
| this.cssFiles.set(href, entry); | |
| entry.promise = new Promise<void>((resolve, reject) => { | |
| link.rel = "stylesheet"; | |
| link.onload = () => resolve(); | |
| link.onerror = reject; | |
| link.href = href; | |
| document.head.appendChild(link); | |
| }).finally(() => { | |
| entry.promise = null; | |
| }); | |
| wait.push(entry.promise); | |
| } |
🤖 Prompt for AI Agents
In packages/bun-framework-react/src/client/css.ts around lines 48 to 64, the
created Promise.finally references entry before entry is assigned, which risks a
TDZ/closure hazard; initialize entry first (e.g., create entry = { promise:
null, link } after creating the link element but before constructing the
Promise) and then assign the Promise to entry.promise so the finally callback
can safely set entry.promise = null; ensure you still set
this.cssFiles.set(href, entry) before pushing the promise to wait.
|
|
||
| // Note: we do not check for `use${Hook}` here because at | ||
| // this stage, we do not yet know if the file we're parsing | ||
| // is exclusively used inside a client component module | ||
| // graph. |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that the server-component guard removal is intentional and has a replacement.
The note indicates that the code no longer checks for use${Hook} at this stage because "we do not yet know if the file we're parsing is exclusively used inside a client component module graph." According to the AI summary, a large conditional block enforcing an error for useState in server components was removed (not shown in the diff).
This removal could introduce a correctness issue if:
- Client-only hooks like useState are allowed in server components without validation elsewhere.
- The deferred check is never implemented or is incomplete.
Run the following script to verify whether there's a replacement check for client-only hooks in server components elsewhere in the codebase:
Additionally, could you confirm:
- Is the removal of the server-component guard intentional?
- Where in the codebase is the deferred check for client-only hooks in server components implemented (if at all)?
- Should the commented-out hook list (lines 36-48) be used for this check?
🏁 Script executed:
#!/bin/bash
# Description: Search for server component validation logic that checks for client-only hooks.
# Search for patterns related to server component validation and client-only hooks.
rg -nP --type=zig -C3 'server.*component|client.*component|useState|useEffect|client.*only.*hook' | head -100Length of output: 5500
🏁 Script executed:
#!/bin/bash
# Print context around line 1452 in visitExpr.zig to verify removal and look for deferred checks.
FILE=$(fd visitExpr.zig)
sed -n '1440,1470p' "$FILE"Length of output: 1714
Ensure server-component hook validation is implemented
No deferred or alternate check for client-only hooks (e.g. useState, useEffect) was found after the guard’s removal. Reintroduce the server-component hook guard here or add a validation pass later in the module-graph resolution to error on client-only hooks in server components.
🤖 Prompt for AI Agents
In src/ast/visitExpr.zig around lines 1452-1456, the removed guard that
prevented client-only hooks (e.g. useState, useEffect, useRef, useLayoutEffect,
etc.) from being used in server components is missing; restore validation by
either reintroducing the hook guard here to detect identifiers matching the
client-only hook names and emit a compile-time error with source location when
the current module/component is a server component, or implement a new
validation pass in module-graph resolution that scans parsed ASTs for these hook
identifiers and errors for any usage inside modules marked as server-only;
ensure the error message names the hook and file/line, and keep the same
semantics as the previous guard (no-op when file is confirmed client-only).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/bake.zig (1)
98-99: LGTM! Exception handling properly added.The call to
scope.returnIfException()at line 99 correctly addresses the concern from the previous review, ensuring any JS exceptions thrown byBakeGetDefaultExportFromModuleare properly surfaced as Zig errors.
🧹 Nitpick comments (3)
src/bake.zig (3)
13-14: Consider adding a Zig wrapper for the extern function.Per coding guidelines, extern C symbols should have a Zig-friendly wrapper method. This would improve type safety and idiomatic usage.
Example wrapper:
// External functions for module loading extern fn BakeGetDefaultExportFromModule(global: *jsc.JSGlobalObject, key: jsc.JSValue) jsc.JSValue; pub fn getDefaultExportFromModule(global: *jsc.JSGlobalObject, key: jsc.JSValue) jsc.JSValue { return BakeGetDefaultExportFromModule(global, key); }As per coding guidelines.
97-97: Remove unused binding.The
resolvedbinding is not used. Replace with_to make the intent clear.- .fulfilled => |resolved| blk: { - _ = resolved; + .fulfilled => |_| blk: {
114-114: Remove unused binding.The
errbinding is not used. Replace with_to make the intent clear.- .rejected => |err| { - _ = err; + .rejected => |_| {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📥 Commits
Reviewing files that changed from the base of the PR and between d7217f26d52f44f9d10dc7b57d4aebdc26afa2b1 and c84adf8eaaaa21a54542aa26b7f5b6e71fed3e51.
📒 Files selected for processing (1)
src/bake.zig(8 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Implement debug logs in Zig using
const log = bun.Output.scoped(.${SCOPE}, false);and invokinglog("...", .{})
Files:
src/bake.zig
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
**/*.zig: Format Zig files with zig-format (bun run zig-format)
In Zig, manage memory carefully with allocators and use defer for cleanup
Files:
src/bake.zig
🧠 Learnings (2)
📚 Learning: 2025-08-30T00:11:57.076Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-08-30T00:11:57.076Z
Learning: Applies to src/{**/js_*.zig,bun.js/api/**/*.zig} : Use bun.JSError!JSValue for proper error propagation in JS-exposed Zig functions
Applied to files:
src/bake.zig
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions
Applied to files:
src/bake.zig
🔇 Additional comments (4)
src/bake.zig (4)
32-32: LGTM! Signature change supports dynamic framework resolution.The addition of the
resolverparameter is necessary for the new dynamic framework resolution feature and is used appropriately throughout the function.
430-434: LGTM! Proper separation of concerns.The added validation and comment correctly reflect that string framework handling has been moved to
UserOptions.fromJS, and this function now expects only object-based framework definitions.
403-413: LGTM! Improved error handling.The added null and empty path checks with descriptive error messages improve robustness and debugging experience.
78-79: EnsurepathConst()never returns null before unwrapping
At src/bake.zig:78,resolved_path.pathConst().?.textwill panic ifpathConst()is null—confirm that resolution always produces a valid path or handle the null case explicitly.
| .path = bun.fs.Path.initForKitBuiltIn("bun", "app/server"), | ||
| .contents = "", // Virtual | ||
| .index = bun.ast.Index.bake_server_data, | ||
| }; | ||
|
|
||
| pub const client_virtual_source: bun.logger.Source = .{ | ||
| .path = bun.fs.Path.initForKitBuiltIn("bun", "bake/client"), | ||
| .path = bun.fs.Path.initForKitBuiltIn("bun", "app/client"), | ||
| .contents = "", // Virtual | ||
| .index = bun.ast.Index.bake_client_data, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify all references to old virtual paths have been updated.
The virtual source paths have been changed from bake/server and bake/client to app/server and app/client. Ensure all references to the old paths throughout the codebase have been updated accordingly.
🏁 Script executed:
#!/bin/bash
# Description: Search for references to old virtual source paths
# Search for old path references
rg -n 'bake/(server|client)' --type=zig --type=js --type=ts
# Search for references to the path constants
rg -n 'server_virtual_source|client_virtual_source' --type=zigLength of output: 895
Update remaining virtual path in codegen
- src/codegen/bake-codegen.ts:56: change
css("../bake/client/overlay.css", !!debug)tocss("../app/client/overlay.css", !!debug).
🤖 Prompt for AI Agents
In src/bake.zig around lines 881-890 there are virtual path entries for
client/server; update the codegen source reference in
src/codegen/bake-codegen.ts (around line 56) where it currently uses
css("../bake/client/overlay.css", !!debug) to instead reference the real virtual
path css("../app/client/overlay.css", !!debug); modify that string only so
codegen points to "../app/client/overlay.css" and keep the rest of the call
(including the !!debug flag) unchanged.
4 participants
What does this PR do?
wip
How did you verify your code works?
wip