Skip to content

bake: prerender routes that have layouts, and keep a route static only if every layout is - #47150

Open
robobun wants to merge 1 commit into
mainfrom
robobun/d5d40a95/bake-prerender-layouts
Open

robobun wants to merge 1 commit into
mainfrom
robobun/d5d40a95/bake-prerender-layouts

Conversation

@robobun

@robobun robobun commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

Problem

  • bun build --app exits 1 with TypeError: No default value when a route has a layout. The react framework has layouts on.
  • renderRoutesForProdStatic (src/js/builtins/Bake.ts:132): the .then parameter shadows the file index, so a module namespace object is the key into loadedModules.
  • Behind it: a route's no_client flag reads only the page (src/runtime/bake/production/mod.rs:1115). A client component in a layout gets no script.

Fix

  • Bake.ts: the callback stores the module under the file index. The parameter type object makes tsc reject it as a key.
  • production/mod.rs: the flag is the AND over the page and each layout above it. A route renders all of them.
  • Verified: test/bake/dev/production.test.ts, 4 new cases. All fail on main. With only the Bake.ts change, 1 fails.
  • Self-reviewed: 19 concerns raised, 17 addressed (Notes).

Background

Downsides

Notes

Source and reach.

Repro. A framework with no dependencies:

app.ts             export default { app: { framework: { fileSystemRouterTypes: [{ root: "routes", style: "nextjs-pages", serverEntryPoint: "./server.ts", layouts: true }] } } };
server.ts          export function render(req, meta) { return new Response(String(meta.pageModule.default())); }
                   export async function prerender(meta) { return { files: { "/index.html": String(meta.pageModule.default()) + " layouts=" + meta.layouts.map(l => l.default()).join(",") } }; }
routes/index.ts    export default () => "home";
routes/_layout.ts  export default () => "_layout.ts";

$ bun build --app ./app.ts        # main, and canary 1.4.4-canary.1+a0cc096ce
Rendering routes
  loaded = fileList.map((x) => loadedModules[x] ?? (anyPromise = !0, import(allServerFiles[x]).then((x) => loadedModules[x] = x)));
TypeError: No default value
$ echo $?
1

Without routes/_layout.ts the build exits 0. With this PR dist/index.html is home layouts=_layout.ts.

Proof runs.

  • Release build of main 9b8347a: 4 of 4 new cases fail, each with TypeError: No default value. A debug build of that commit fails the same way.
  • Rename only (a canary with the two bytes of the rename patched): 3 cases pass. The server components case fails: /blog/admin, /docs, /docs/guide and /docs/intro get 0 modules. A debug build without the fully_static &= line fails the same case.
  • This PR, debug build, rebased on 11c3671: 4 of 4 pass. The whole file: 25 pass with --timeout 180000. The React cases need more than 5 s on a debug build, on main too.
  • tsc --noEmit -p src/js/tsconfig.json: no error in Bake.ts. With loadedModules[mod] = mod it reports Bake.ts(134,97): error TS2538: Type 'object' cannot be used as an index type.
  • The react framework, run by hand with the two cases of e47140f (a root layout and a nested layout, a client component in a page and in a layout): main fails both with the TypeError. Rename only passes the first and fails the second, because /docs and /docs/intro have no <script type="module">. This PR passes both.
  • Also on that debug build: framework-router, app-options, dev-and-prod, dev/ssg-pages-router and dev/react-spa in test/bake (82 pass).

What the new cases cover. Both use a framework with no dependencies that writes, for each page, the files, the count of client modules and the stylesheets that the build passed.

  • Case 1, for nextjs-pages and nextjs-app-ui: a root layout, a nested layout that two pages share, a page two levels below a layout, and a dynamic route. Each page gets its layouts with the inner-most first. getParams gets the same layouts. Each page gets the stylesheet that the root layout imports.
  • Case 2, with and without server components: no layout, one static layout, two static layouts, a client component in the page, in the layout of the route, in the layout of the parent route, in the inner layout of two, in the outer layout of two. Without server components every page keeps the client entry point.

Still wrong for a build with a layout. Main fails each of these builds with the TypeError. With this PR they reach a defect that main already has for routes with no layout. I ran each one on release builds of main and of this PR.

  1. Two layouts for one route pattern (_layout.ts beside _layout.js): the build prints error: Multiple layout matching the same route pattern is ambiguous, keeps one layout and exits 0. Two pages for one pattern do the same on main. bake: fail bun build --app when two files match the same route pattern #47146 fails the build for every collision.
  2. An onResolve plugin that passes on a route file: pages are built from the modules of other files, exit 0. Main does this for routes with no layout. bake: bind each production entry point to its graph node, not to its position #46804 binds each entry point to its own output.
  3. A prerender that fails for a route from getParams: the build prints the error and done, and exits 0 without the page. This happens under { pages }, and under an iterator when prerender is not async. Main does this for routes with no layout. bake: fail the build when a component throws during static prerender #33216 changes the { pages } arm.
  4. A stylesheet that a page and its layout both import is in meta.styles twice. The array also has a hole for each stylesheet of the app that the route does not use. The hole is on main for routes with no layout. 771bda2 in bake: consolidate the open robobun dev-server, HMR runtime, production build and router fixes #39488 removes the duplicate.
  5. A route file that reaches a client component only through an import cycle can stay static (StaticRouteVisitor.rs:27). Main does this for pages.
  6. A page or a layout that is itself a "use client" file still exits 1 with Cannot create a string longer than 2147483647 characters, with or without a layout, as on main. bake: bind each production entry point to its graph node, not to its position #46804 names the cause in its error.

Other open PRs on these lines.

Relation to the Bake rework. This PR repairs the prerender step of bun build --app, the only production build on main. #46018 (bun bake build) closed on 2026-10-21 "while the Bake design is reworked". An open question asks whether the prerender step in build_with_vm stays: #46804 (comment). If it does not stay, this PR can close with nothing left over.

Designs considered.

  • The rename alone. It fixes the TypeError. Then 2 of 4 routes of a fixture with a client component in a layout get no client entry point, at exit 0.
  • The fold of bake: prerender routes that have layouts #38336, in the fill pass: two &= sites in a loop body that exists three times, and bake: bind each production entry point to its graph node, not to its position #46804 rewrites that loop. The walk that counts the layouts visits each one once.
  • One loader for the single-file and the multi-file branch: +3 allocations and +1 microtask for each route with no layout, and it rewrites the branch that works.
  • One generated server entry point for each route: it changes the output of builds that pass today.
  • The framework decides at render time, as in the dev server, which always passes the client entry point. This changes meta.modules for every server components framework and removes StaticRouteVisitor. It needs a maintainer's decision.
  • An oxlint no-shadow rule: it flags 175 sites in src/js, and one of them is a bug.

Measurements. Release builds (LTO) and debug builds of main 9b8347a and of this change on that commit. The rebase on 11c3671 changed no line of production/ and only type annotations in Bake.ts. bloaty, perf, valgrind and strace are not installed in the build container.

  • Builtin source: +8 bytes (N in WebCoreJSBuiltins.cpp: debug 5735 to 5743, release 3552 to 3560). The object annotation emits 0 bytes.
  • Release binary: build_with_vm +63 bytes (nm -S, 12,959 to 13,022). .text (83,497,513), .rodata (22,574,028) and the stripped binary (109,037,072 bytes) have equal sizes.
  • Route with no layout: +1 instruction and +1 conditional branch per route-chain node (objdump: add 0x38(%r15),%r14d becomes cmpb $0x0,0x38(%r15) and je). A build of 1,000 pages at depth 3: 323,013,641 and 321,667,711 user instructions (medians of 5 interleaved runs, perf_event_open counter). The spread between runs is 7.8 million or more, so the 4,000 instructions that the static count predicts do not show.
  • PerThread::output_file calls (gdb hit counts): +0 for a route with no layout (2 and 2), +1 for each layout above a route (18 to 26 for 8 pairs of a route and a layout).
  • JS work for a route with no layout: +0 microtask turns (13 and 13), +0 import() calls (1 and 1). The fixture of case 1: 7 import() calls for 7 route files.
  • Builds that pass on main: 0 files differ in dist/ (diff -r). The fixtures have 1, 8 and 1,000 pages, and one has server components and a client entry point.
  • Syscalls of a build with no layout (ptrace counter, 15 interleaved runs each): read 19, write 9, openat 26, close 24, getdents64 16 on every run of both. Totals are 555 to 608 and 549 to 590, and the spread is futex.
  • no_client: 2 of 4 routes change against a rename-only build (/docs, /docs/intro). Without server components 0 of 4 change.

Self-review. 19 concerns.

…y if every layout is

`bun build --app` failed with `TypeError: No default value` for a route
that has a layout. In `renderRoutesForProdStatic` the `.then` parameter
shadowed the file index, so the module namespace object was the key
into `loadedModules`. The callback now stores the module under the file
index.

The `no_client` flag of a route came from the page file alone. Each
layout is a separate server entry point with its own classification, so
a static page under a layout that imports a client component got no
client entry point. The flag is now folded over the page and every
layout above it, in the walk that counts those layouts.
@robobun

robobun commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 4:06 AM PT - Oct 28th, 2026

❌ @robobun, your commit ba5ba81 has 2 failures in Build #134300 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 47150

That installs a local version of the PR into your bun-47150 executable, so you can run:

bun-47150 --bun

@robobun

robobun commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready for review. The diff is green on every CI lane that ran.

How I reproduced the bug:

  • A release build of main (9b8347a) and canary 1.4.4-canary.1+a0cc096ce: bun build --app ./app.ts with routes/index.ts and routes/_layout.ts exits 1 with TypeError: No default value. Without the layout the build exits 0.
  • The 4 new cases in test/bake/dev/production.test.ts fail on that build, each with the same error.
  • With only the Bake.ts change, 3 cases pass. The server components case fails: four routes under a layout that imports a client component get no client entry point.
  • With this PR, all 4 cases pass on a debug build of the branch (rebased on 11c3671).

CI for ba5ba81 (build 134300):

  • Every test lane that ran passed: Debian, Ubuntu and Alpine (x64 and aarch64), Debian x64 with ASAN, Windows (x64 and aarch64), and macOS aarch64. The ASAN lane ran test/bake/dev/production.test.ts: 25 pass, 0 fail. The macOS aarch64 lane ran its 25 tests too.
  • The macOS x64 test lane did not run, and that is the only reason the build is marked failed. Its two jobs expired four times with no agent assigned. No macOS x64 test job started in any of the 40 most recent builds of the pipeline (09:18 to 11:12 UTC on 2026-10-28). The macOS x64 build job passed.
  • Two shards of the test-aot-drawn lane failed: test/aot/minifying-first-saves-no-code and test/aot/perf/generated-code-of-idioms. That lane is allowed to fail. This PR does not touch that code, and the same two test files fail on main at the base commit 11c3671 (build 134092).

PR: #47150

@coderabbitai

coderabbitai Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 11c3671 and ba5ba81.

Files selected for processing: 3
  • src/js/builtins/Bake.ts
  • src/runtime/bake/production/mod.rs
  • test/bake/dev/production.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

Walkthrough

Production route flags now account for the page and its layouts. New tests cover layout chains, prerendered styles, and client-bundle counts. The module-loading callback also uses renamed parameters without changing its behavior.

Changes

Static route classification

Layer / File(s) Summary
Aggregate page and layout classifications
src/runtime/bake/production/mod.rs
The route flag starts with the page classification, combines the classification of each present layout, and passes the aggregate value to TypeAndFlags.
Production-build coverage
test/bake/dev/production.test.ts
Tests check layout chains, getParams-derived routes, prerendered styles, and module counts with server components enabled and disabled.

Module loader callback naming

Layer / File(s) Summary
Rename callback parameters
src/js/builtins/Bake.ts
The callback parameters change from x to id and mod. Loading and caching behavior remain unchanged.

Suggested reviewers: jarred-sumner

Priority: Normal

Merge Risk: Minimal · up to ba5ba

Routes with layouts now use the intended module and classification paths. No merge-blocking risk was identified.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly identifies both primary changes: prerendering routes with layouts and requiring all layouts to be static. The final wording is grammatically incomplete, but the intended change is cl…
Description check Passed The description thoroughly explains the problem, fix, verification, scope, limitations, and related issues. It does not use the template headings exactly, but it provides the required information in e…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title clearly identifies both primary changes: prerendering routes with layouts and requiring all layouts to be static. The final wording is grammatically incomplete, but the intended change is clear.

Full details: Description check

Explanation

The description thoroughly explains the problem, fix, verification, scope, limitations, and related issues. It does not use the template headings exactly, but it provides the required information in equivalent sections.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR

@claude claude 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.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked that the new fully_static walk in build_with_vm (src/runtime/bake/production/mod.rs:992-1011) visits exactly the same layout chain the fill pass loads into file_list (route.file_layout, then each parent's), so the flag covers every module the route actually renders; and that src/js/builtins/Bake.ts has no ahead-of-time/ twin needing the same rename.

Extended reasoning...

The change is a two-line variable-shadowing fix in the Bake prerender builtin, a small Rust accumulator that ANDs each layout's static classification into the route's flag, and a new test block; it touches no auth, crypto, or input-parsing surface. Two confirmed findings are posted inline, so a human look is already signalled; this note records only what else was verified.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

  • 🟣 src/js/builtins/Bake.ts — pre-existing: framework users whose prerender fails for a dynamic route under a layout now get exit 0 and a missing page instead of the loud exit 1 the base gives every layout build. In the { pages } arm at Bake.ts:176 the arrow has a block body, so the promise from callRouteGenerator is dropped and Promise.all resolves on undefined. The iterator arms only await a result that is still pending, so a synchronously throwing prerender (already-rejected promise) is dropped too. Fix: propagate every rejection from callRouteGenerator to the render promise on all three arms, e.g. return the promise in the map and await result unconditionally. Same pattern at 3 sites (Bake.ts:162, Bake.ts:169, Bake.ts:176).

    Why this was flagged

    A route with params under a layout, whose getParams returns { pages: [...] }, and whose prerender rejects for one of the params. On the base branch the build never reaches this arm for any route with a layout: renderRoutesForProdStatic threw TypeError: No default value at the shadowed loadedModules[x] and exited 1. After this PR the route reaches Bake.ts:174-178, where pages.map(params => { callRouteGenerator(...); }) returns undefined for each entry, so await Promise.all(...) resolves immediately and the rejected doGenerateRoute promise is an unhandled rejection. build_with_vm then sees Unwrapped::Fulfilled at src/runtime/bake/production/mod.rs:1184, prints done, and returns Ok(()), so the caller at mod.rs:223 leaves exit_code 0 with that page's files missing from dist. The iterator arms at Bake.ts:162 and Bake.ts:169 gate on $isPromisePending(result); when the framework prerender throws synchronously, doGenerateRoute returns an already-rejected promise, the gate is false, and the rejection is dropped the same way.

    Verification: pre-existing (the base already fails the same way for routes without layouts; this PR newly routes layout builds onto the same arm). In src/js/builtins/Bake.ts:174-178 the arrow has a block body with no return, so Promise.all resolves immediately and the promise from doGenerateRoute is dropped. build_with_vm returns Ok(()), so the build exits 0 with the page's files never written.

Comment thread src/runtime/bake/production/mod.rs Resolved
@robobun

robobun commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

The additional finding is correct: a failed prerender of a route from getParams can end in exit 0. This PR does not change those lines.

  • Main has the defect for a route with no layout. I ran all three arms on a release build of main: { pages }, an array and an async generator, the last two with a prerender that is not async and throws. Each run prints the error and done, exits 0 and leaves the page out. In the iterator arms an async prerender that rejects exits 1.
  • For a route under a layout, main stops at the TypeError before it reaches these arms. With this PR the build reaches them and behaves as main does with no layout. The Notes of the PR list this as case 3 under "Still wrong for a build with a layout".
  • The fix changes the exit code of builds that exit 0 on main, so it is not in this PR. bake: fail the build when a component throws during static prerender #33216 returns the promise in the { pages } arm. Commit 80892c6 in bake: consolidate the open robobun dev-server, HMR runtime, production build and router fixes #39488 awaits every result in the two iterator arms.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant