feat(start): add pre-render params + sitemap#7346
Draft
nikuscs wants to merge 9 commits intoTanStack:mainfrom
Draft
feat(start): add pre-render params + sitemap#7346nikuscs wants to merge 9 commits intoTanStack:mainfrom
nikuscs wants to merge 9 commits intoTanStack:mainfrom
Conversation
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds typed route-level prerenderParams and runtime to collect and run them at build time: type augments, route-tree discovery, a runner with abort/timeout/merge/dedup logic, sitemap output enhancements, client stripping of server-only markers, docs, e2e examples, and tests across React/Solid/Vue and core packages. ChangesDynamic Route Prerendering
Sequence Diagram(s)sequenceDiagram
actor Build as Build Process
participant RouterGen as Router Generator
participant RouteTree as Built Server RouteTree
participant Runner as runPrerenderParams
participant PreparamFn as prerenderParams Callback
participant PageGen as Page Interpolator
participant Sitemap as buildSitemap
Build->>RouterGen: generate route metadata
RouterGen->>globalThis: set TSS_PRERENDER_DYNAMIC_ROUTES
Build->>RouteTree: import built server entry
RouteTree-->>Build: expose routeTree via TSS_PRERENDER_ROUTE_TREE()
Build->>Runner: runPrerenderParams({ routeTree, pages, timeout })
Runner->>PreparamFn: invoke({ routePath, signal })
PreparamFn-->>Runner: return entries[]
Runner->>PageGen: interpolate params, encode path, serialize search
PageGen-->>Runner: emit Page objects
Runner->>Runner: merge, dedupe, apply sitemap/prerender options
Runner-->>Build: return final pages
Build->>Sitemap: buildSitemap(final pages)
Sitemap->>Sitemap: apply per-entry metadata & namespaces
Sitemap-->>Build: emit sitemap.xml
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
github-actions
Bot
added
documentation
Contributor
|
View your CI Pipeline Execution ↗ for commit 1c27261
☁️ Nx Cloud last updated this comment at |
Contributor
Bundle Size Benchmarks
Trend sparkline is historical gzip bytes ending with this PR measurement; lower is better. |
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
e2e/vue-start/basic/rsbuild.config.ts (1)
8-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
sitemapoption to rsbuild prerender config for feature parity with viteThe Vue
vite.config.tsconditionally enablessitemapgeneration whenisPrerenderis true, butrsbuild.config.tsomits this option. The prerendering test includes sitemap assertions, but usestest.skip()to gracefully skip the test if the sitemap is missing — so this won't cause a test failure. However, it creates an inconsistency where users building with rsbuild won't getsitemap.xmloutput during prerendering, unlike the vite build.🛠️ Proposed fix (mirror the vite config)
const prerenderConfiguration = { ... } export default defineConfig({ plugins: [ pluginBabel({ include: /\.(?:jsx|tsx)$/ }), pluginVue(), pluginVueJsx(), - tanstackStart({ - prerender: isPrerender ? prerenderConfiguration : undefined, - }), + tanstackStart({ + prerender: isPrerender ? prerenderConfiguration : undefined, + sitemap: isPrerender + ? { enabled: true, host: 'https://example.com' } + : undefined, + }), ],🤖 Prompt for 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. In `@e2e/vue-start/basic/rsbuild.config.ts` around lines 8 - 37, The rsbuild prerender config omits the sitemap option so rsbuild builds won't produce sitemap.xml; update the prerender configuration passed to tanstackStart in rsbuild.config.ts by adding sitemap: true when prerendering is enabled (e.g., extend prerenderConfiguration or the object passed to tanstackStart to include sitemap: true when isPrerender is true), referencing prerenderConfiguration, tanstackStart and isPrerender so the rsbuild prerender behavior matches the vite config.
🧹 Nitpick comments (5)
e2e/solid-start/basic/rsbuild.config.ts (2)
7-23: 💤 Low valueDuplicate prerender filter list with
vite.config.ts— consider extracting a shared config helperThe
prerenderConfiguration(filter list +maxRedirects) defined here is byte-for-byte identical to theprerenderConfigurationine2e/solid-start/basic/vite.config.ts. The React e2e app already centralizes this ine2e/react-start/basic/start-mode-config.tsviagetStartModeConfig(). Applying the same pattern for Solid (and Vue) prevents silent filter-list drift when new routes are added.♻️ Proposed refactor — extract a shared start-mode-config.ts for Solid
Create
e2e/solid-start/basic/start-mode-config.ts:import { isPrerender } from './tests/utils/isPrerender' import { isSpaMode } from './tests/utils/isSpaMode' export function getStartModeConfig() { return { spa: isSpaMode ? { enabled: true, prerender: { outputPath: 'index.html' } } : undefined, prerender: isPrerender ? { enabled: true, filter: (page: { path: string }) => ![ '/this-route-does-not-exist', '/redirect', '/i-do-not-exist', '/not-found', '/specialChars/search', '/specialChars/hash', '/specialChars/malformed', '/search-params/default', '/transition', '/users', ].some((p) => page.path.includes(p)), maxRedirects: 100, } : undefined, sitemap: isPrerender ? { enabled: true, host: 'https://example.com' } : undefined, } }Then in both
vite.config.tsandrsbuild.config.ts:-import { isPrerender } from './tests/utils/isPrerender' +import { getStartModeConfig } from './start-mode-config' -const prerenderConfiguration = { ... } ... -tanstackStart({ prerender: isPrerender ? prerenderConfiguration : undefined, sitemap: ... }) +tanstackStart(getStartModeConfig())🤖 Prompt for 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. In `@e2e/solid-start/basic/rsbuild.config.ts` around lines 7 - 23, The prerenderConfiguration object (including filter and maxRedirects) is duplicated; extract it into a shared getStartModeConfig() helper (like the React approach) that returns the prerender config (with enabled, filter function, maxRedirects and sitemap/spa variants) and import that helper into both vite.config.ts and rsbuild.config.ts, then replace the local prerenderConfiguration constant with the shared value (referencing getStartModeConfig and its prerender property) so both build configs use the same filter array and settings.
7-35: ⚡ Quick win
sitemapoption missing from rsbuild prerender config — feature parity gap
e2e/solid-start/basic/vite.config.tsincludessitemap: { enabled: true, host: 'https://example.com' }whenisPrerenderis true, but the rsbuild config does not. While the sitemap test inprerendering.spec.ts(line 147) includes atest.skip()guard that gracefully skips when the sitemap is disabled, the inconsistency means the rsbuild build lacks this feature compared to vite. Consider adding the sitemap option for parity:Optional enhancement
tanstackStart({ prerender: isPrerender ? prerenderConfiguration : undefined, + sitemap: isPrerender + ? { enabled: true, host: 'https://example.com' } + : undefined, }),🤖 Prompt for 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. In `@e2e/solid-start/basic/rsbuild.config.ts` around lines 7 - 35, The rsbuild config is missing the sitemap option for prerender parity; update the tanstackStart call in rsbuild.config.ts (where tanstackStart({ prerender: isPrerender ? prerenderConfiguration : undefined })) so that when isPrerender is true the prerender config includes sitemap: { enabled: true, host: 'https://example.com' } (either by adding sitemap to the existing prerenderConfiguration object or by merging it when passing into tanstackStart).packages/start-client-core/src/prerenderParams.ts (1)
61-90: ⚡ Quick winDocument expected date string formats.
lastmodandnews.publicationDateacceptstring | Date, but the sitemap writer normalizes toYYYY-MM-DD(perbuild-sitemap.test.ts). Adding short JSDoc on these fields (e.g., "ISO 8601 date orYYYY-MM-DD;Dateis formatted to W3C date format") would prevent users from passing arbitrary strings that get emitted verbatim into the XML.📝 Proposed JSDoc additions
alternateRefs?: Array<{ href: string hreflang: string }> + /** + * Last modification date. Accepts a `Date` (formatted as `YYYY-MM-DD`) + * or a W3C/ISO 8601 date string. Strings are emitted verbatim, so + * pass `YYYY-MM-DD` or full ISO 8601 to remain spec-compliant. + */ lastmod?: string | Date ... news?: { publication: { name: string language: string } + /** + * Publication date. `Date` is formatted as `YYYY-MM-DD`; + * strings should be W3C/ISO 8601 to remain spec-compliant. + */ publicationDate: string | Date title: string }🤖 Prompt for 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. In `@packages/start-client-core/src/prerenderParams.ts` around lines 61 - 90, Update the RouteSitemapOptions interface JSDoc to document expected date string formats for lastmod and news.publicationDate: add brief comments on the lastmod property and the nested news.publicationDate explaining they accept ISO 8601 strings or YYYY-MM-DD and that Date values will be formatted to W3C/ISO (normalized to YYYY-MM-DD by the sitemap writer). Reference the RouteSitemapOptions interface and its lastmod and news.publicationDate fields so consumers know strings will be emitted verbatim unless they follow the documented formats.packages/start-plugin-core/tests/prerender-routes-plugin.test.ts (1)
39-69: 💤 Low valueLGTM – consider adding a
TSS_PRERENDER_DYNAMIC_ROUTESassertion to test 2.Test 2 verifies that
/posts/$slug(which hascomponentbut notprerenderParams) stays out ofTSS_PRERENDABLE_PATHS, but doesn't confirm it's also absent fromTSS_PRERENDER_DYNAMIC_ROUTES. Test 3 covers this indirectly, so it's not a bug — just a small coverage gap.✨ Optional assertion to make test 2 self-contained
expect(globalThis.TSS_PRERENDABLE_PATHS).toEqual([{ path: '/' }]) + expect(globalThis.TSS_PRERENDER_DYNAMIC_ROUTES).toEqual([]) })🤖 Prompt for 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. In `@packages/start-plugin-core/tests/prerender-routes-plugin.test.ts` around lines 39 - 69, Add an assertion in the test "does not store API, layout, or dynamic routes as static paths" to also verify that dynamic routes without prerenderParams are not recorded in TSS_PRERENDER_DYNAMIC_ROUTES: after invoking plugin.onRouteTreeChanged (using prerenderRoutesPlugin and the route node with routePath '/posts/$slug'), assert that globalThis.TSS_PRERENDER_DYNAMIC_ROUTES does not contain an entry for '/posts/$slug' (or is empty), so the test checks both TSS_PRERENDABLE_PATHS and globalThis.TSS_PRERENDER_DYNAMIC_ROUTES consistency.packages/start-plugin-core/src/vite/prerender.ts (1)
38-50: ⚡ Quick winAddress the fragile output filename assumption.
The code computes the server output filename as
${basename(serverInput, extname(serverInput))}.js, which assumes the built file uses the same base name as the input and always ends in.js. A project that setsrollupOptions.output.entryFileNameswith a hash pattern or emits.cjs/.mjswould produce anERR_MODULE_NOT_FOUNDcrash at prerender time. Consider reading from a Vite manifest or build metadata to get the actual output filename instead of computing it.🤖 Prompt for 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. In `@packages/start-plugin-core/src/vite/prerender.ts` around lines 38 - 50, The code that builds serverEntryPath from serverInput (using basename + ".js") is fragile for hashed or non-.js outputs; instead look up the actual built entry filename from the Vite/Rollup manifest or build output metadata and use that to form serverEntryPath. Concretely: when preparing to import the built server entry, read the build manifest (or Rollup output info) in the same output directory returned by getServerOutputDirectory(serverEnv.config), find the entry whose source or file corresponds to serverInput (use serverInput as the original input key), and use the manifest-provided filename (which may be .cjs/.mjs or hashed) for import; update the logic around serverInput, serverOutputDir, serverEntryPath and the await import(...) to use the manifest-resolved filename instead of `${basename(...)}.js`.
🤖 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 `@e2e/vue-start/basic/tests/prerendering.spec.ts`:
- Line 119: The assertion expect(html).toContain('2') is too broad; update the
test to assert the specific element or text that indicates the search param
page=2 was rendered instead of any occurrence of the character '2'. Replace the
loose expect(html).toContain('2') check (referencing the html variable and the
expect call) with a targeted assertion—either assert the exact HTML substring
for the element that should show "2" (e.g., the specific heading or pager
markup) or parse html with a DOM parser (e.g., JSDOM) and use querySelector to
assert the element's textContent equals '2' (or matches a precise regex like
/\bpage[:\s]*2\b/).
In `@packages/start-plugin-core/src/prerender-params-runner.ts`:
- Around line 131-142: The promise executor currently checks signal.aborted once
but then schedules callback via Promise.resolve().then(callback) allowing a
race; change the scheduled invocation to first re-check signal.aborted and
throw/reject if set before calling callback (e.g. replace .then(callback) with
.then(() => { if (signal.aborted) throw signal.reason ?? new
Error('prerenderParams aborted'); return callback(); })) and ensure the abort
listener (abort) is removed/cleanup occurs after resolve/reject so the handler
and callback cannot both run; reference the Promise executor, signal, callback,
and abort variables in prerender-params-runner.ts.
---
Outside diff comments:
In `@e2e/vue-start/basic/rsbuild.config.ts`:
- Around line 8-37: The rsbuild prerender config omits the sitemap option so
rsbuild builds won't produce sitemap.xml; update the prerender configuration
passed to tanstackStart in rsbuild.config.ts by adding sitemap: true when
prerendering is enabled (e.g., extend prerenderConfiguration or the object
passed to tanstackStart to include sitemap: true when isPrerender is true),
referencing prerenderConfiguration, tanstackStart and isPrerender so the rsbuild
prerender behavior matches the vite config.
---
Nitpick comments:
In `@e2e/solid-start/basic/rsbuild.config.ts`:
- Around line 7-23: The prerenderConfiguration object (including filter and
maxRedirects) is duplicated; extract it into a shared getStartModeConfig()
helper (like the React approach) that returns the prerender config (with
enabled, filter function, maxRedirects and sitemap/spa variants) and import that
helper into both vite.config.ts and rsbuild.config.ts, then replace the local
prerenderConfiguration constant with the shared value (referencing
getStartModeConfig and its prerender property) so both build configs use the
same filter array and settings.
- Around line 7-35: The rsbuild config is missing the sitemap option for
prerender parity; update the tanstackStart call in rsbuild.config.ts (where
tanstackStart({ prerender: isPrerender ? prerenderConfiguration : undefined }))
so that when isPrerender is true the prerender config includes sitemap: {
enabled: true, host: 'https://example.com' } (either by adding sitemap to the
existing prerenderConfiguration object or by merging it when passing into
tanstackStart).
In `@packages/start-client-core/src/prerenderParams.ts`:
- Around line 61-90: Update the RouteSitemapOptions interface JSDoc to document
expected date string formats for lastmod and news.publicationDate: add brief
comments on the lastmod property and the nested news.publicationDate explaining
they accept ISO 8601 strings or YYYY-MM-DD and that Date values will be
formatted to W3C/ISO (normalized to YYYY-MM-DD by the sitemap writer). Reference
the RouteSitemapOptions interface and its lastmod and news.publicationDate
fields so consumers know strings will be emitted verbatim unless they follow the
documented formats.
In `@packages/start-plugin-core/src/vite/prerender.ts`:
- Around line 38-50: The code that builds serverEntryPath from serverInput
(using basename + ".js") is fragile for hashed or non-.js outputs; instead look
up the actual built entry filename from the Vite/Rollup manifest or build output
metadata and use that to form serverEntryPath. Concretely: when preparing to
import the built server entry, read the build manifest (or Rollup output info)
in the same output directory returned by
getServerOutputDirectory(serverEnv.config), find the entry whose source or file
corresponds to serverInput (use serverInput as the original input key), and use
the manifest-provided filename (which may be .cjs/.mjs or hashed) for import;
update the logic around serverInput, serverOutputDir, serverEntryPath and the
await import(...) to use the manifest-resolved filename instead of
`${basename(...)}.js`.
In `@packages/start-plugin-core/tests/prerender-routes-plugin.test.ts`:
- Around line 39-69: Add an assertion in the test "does not store API, layout,
or dynamic routes as static paths" to also verify that dynamic routes without
prerenderParams are not recorded in TSS_PRERENDER_DYNAMIC_ROUTES: after invoking
plugin.onRouteTreeChanged (using prerenderRoutesPlugin and the route node with
routePath '/posts/$slug'), assert that globalThis.TSS_PRERENDER_DYNAMIC_ROUTES
does not contain an entry for '/posts/$slug' (or is empty), so the test checks
both TSS_PRERENDABLE_PATHS and globalThis.TSS_PRERENDER_DYNAMIC_ROUTES
consistency.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b58420f7-00a8-4fa8-b77b-ab44168b7d58
📒 Files selected for processing (48)
.changeset/tall-trees-prerender-params.mddocs/start/framework/react/guide/static-prerendering.mddocs/start/framework/solid/guide/static-prerendering.mde2e/react-start/basic/src/routeTree.gen.tse2e/react-start/basic/src/routes/-prerender-params.server.tse2e/react-start/basic/src/routes/_layout/_layout-2/prerender-nested.$slug.tsxe2e/react-start/basic/src/routes/prerender-params.$slug.tsxe2e/react-start/basic/start-mode-config.tse2e/react-start/basic/tests/prerendering.spec.tse2e/solid-start/basic/package.jsone2e/solid-start/basic/rsbuild.config.tse2e/solid-start/basic/src/routeTree.gen.tse2e/solid-start/basic/src/routes/-prerender-params.server.tse2e/solid-start/basic/src/routes/_layout/_layout-2/prerender-nested.$slug.tsxe2e/solid-start/basic/src/routes/prerender-params.$slug.tsxe2e/solid-start/basic/tests/prerendering.spec.tse2e/solid-start/basic/vite.config.tse2e/vue-start/basic/package.jsone2e/vue-start/basic/rsbuild.config.tse2e/vue-start/basic/src/routeTree.gen.tse2e/vue-start/basic/src/routes/-prerender-params.server.tse2e/vue-start/basic/src/routes/_layout/_layout-2/prerender-nested.$slug.tsxe2e/vue-start/basic/src/routes/prerender-params.$slug.tsxe2e/vue-start/basic/tests/prerendering.spec.tse2e/vue-start/basic/vite.config.tspackages/start-client-core/src/index.tsxpackages/start-client-core/src/prerenderParams.tspackages/start-client-core/src/tests/prerenderParams.test-d.tspackages/start-plugin-core/src/build-sitemap.tspackages/start-plugin-core/src/global.d.tspackages/start-plugin-core/src/post-build.tspackages/start-plugin-core/src/prerender-params-runner.tspackages/start-plugin-core/src/prerender-route-options.tspackages/start-plugin-core/src/prerender.tspackages/start-plugin-core/src/rsbuild/post-build.tspackages/start-plugin-core/src/rsbuild/start-router-plugin.tspackages/start-plugin-core/src/schema.tspackages/start-plugin-core/src/start-router-plugin/constants.tspackages/start-plugin-core/src/start-router-plugin/generator-plugins/prerender-routes-plugin.tspackages/start-plugin-core/src/vite/prerender.tspackages/start-plugin-core/src/vite/start-router-plugin/plugin.tspackages/start-plugin-core/tests/build-sitemap.test.tspackages/start-plugin-core/tests/prerender-params-runner.test.tspackages/start-plugin-core/tests/prerender-routes-plugin.test.tspackages/start-plugin-core/tests/prerender-ssrf.test.tspackages/start-plugin-core/tests/start-router-plugin-constants.test.tspackages/start-server-core/src/createStartHandler.tspackages/start-server-core/src/global.d.ts
Merging this PR will not alter performance
Comparing 
Footnotes
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/start-plugin-core/src/prerender-params-runner.ts (2)
68-89: 💤 Low valueDefensive validation for non-iterable
entries.
for (const entry of entries)will throw a non-descriptiveTypeError: entries is not iterableif a user-authoredprerenderParamsmistakenly returnsundefined/nullor a non-array. A small guard with a clearer error message attributing the offending route improves DX without changing the happy path.♻️ Suggested guard
const entries = await call( () => options.prerenderParams!({ routePath: route.routePath, signal: controller.signal, }), controller.signal, ).finally(cleanupTimeout) + if (!Array.isArray(entries)) { + throw new Error( + `prerenderParams for route ${route.routePath} must return an array; got ${typeof entries}`, + ) + } + for (const entry of entries) {🤖 Prompt for 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. In `@packages/start-plugin-core/src/prerender-params-runner.ts` around lines 68 - 89, The loop assumes the result of calling options.prerenderParams via call(...) is iterable; add a defensive validation after the await that checks entries is an array (or at least iterable) and throw a clear Error that includes route.routePath (or skip non-iterable with a logged warning) so users get a descriptive message instead of "entries is not iterable"; update the block around the awaited call in prerender-params-runner.ts (where entries is assigned from call(...), which uses controller.signal and options.prerenderParams) to validate entries before the for (const entry of entries) loop and only proceed to call create(...), filter, pagesByPath.set(...), and merge(...) when entries is valid.
155-186: 💤 Low valueRoute-level
options.prerenderis silently dropped on generated pages.
create()mergesoptions.sitemapwithentry.sitemapbut assignsprerender: entry.prerenderdirectly, ignoringoptions.prerenderentirely. If a route declaresprerender: { headers: { … } }at the route level and an entry omitsprerender, those headers are lost on the generated page even though the analogous sitemap config is preserved. If this asymmetry is intentional (e.g., prerender options are applied later in the pipeline), a brief comment would help; otherwise consider symmetric merging.♻️ Symmetric merge sketch
return { path: interpolatedPath + search(entry.search), sitemap: sitemap(options.sitemap, entry.sitemap), - prerender: entry.prerender, + prerender: prerender(options.prerender, entry.prerender), }🤖 Prompt for 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. In `@packages/start-plugin-core/src/prerender-params-runner.ts` around lines 155 - 186, The create function currently drops route-level prerender options by returning prerender: entry.prerender instead of merging with options.prerender; update create (in prerender-params-runner.ts) to merge options.prerender with entry.prerender (similar to how sitemap(options.sitemap, entry.sitemap) is merged) so route-level defaults (e.g., headers) are preserved when entry.prerender is absent—either call an existing merge helper or shallow/deep-merge options.prerender and entry.prerender and return the merged result under prerender.
🤖 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.
Nitpick comments:
In `@packages/start-plugin-core/src/prerender-params-runner.ts`:
- Around line 68-89: The loop assumes the result of calling
options.prerenderParams via call(...) is iterable; add a defensive validation
after the await that checks entries is an array (or at least iterable) and throw
a clear Error that includes route.routePath (or skip non-iterable with a logged
warning) so users get a descriptive message instead of "entries is not
iterable"; update the block around the awaited call in
prerender-params-runner.ts (where entries is assigned from call(...), which uses
controller.signal and options.prerenderParams) to validate entries before the
for (const entry of entries) loop and only proceed to call create(...), filter,
pagesByPath.set(...), and merge(...) when entries is valid.
- Around line 155-186: The create function currently drops route-level prerender
options by returning prerender: entry.prerender instead of merging with
options.prerender; update create (in prerender-params-runner.ts) to merge
options.prerender with entry.prerender (similar to how sitemap(options.sitemap,
entry.sitemap) is merged) so route-level defaults (e.g., headers) are preserved
when entry.prerender is absent—either call an existing merge helper or
shallow/deep-merge options.prerender and entry.prerender and return the merged
result under prerender.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e8757849-427a-4490-af73-adf1d0583e55
📒 Files selected for processing (6)
e2e/react-start/basic/tests/prerendering.spec.tse2e/solid-start/basic/tests/prerendering.spec.tse2e/vue-start/basic/tests/prerendering.spec.tspackages/start-plugin-core/src/post-build.tspackages/start-plugin-core/src/prerender-params-runner.tspackages/start-plugin-core/tests/post-server-build.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- e2e/react-start/basic/tests/prerendering.spec.ts
- e2e/solid-start/basic/tests/prerendering.spec.ts
- e2e/vue-start/basic/tests/prerendering.spec.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/start-plugin-core/src/post-build.ts (1)
17-18: 💤 Low value
spaOnlyis inferred asboolean | undefinedrather thanbooleanWhen
startConfig.spaisundefined, the?.enabledshort-circuits the&&toundefined(notfalse). All current usages (if (spaOnly), ternary) treat it as a falsy check, so there's no runtime defect, but the inferred typeboolean | undefinedis weaker than needed under strict mode.♻️ Suggested refinement
- const spaOnly = - startConfig.spa?.enabled && startConfig.prerender?.enabled !== true + const spaOnly = + !!startConfig.spa?.enabled && startConfig.prerender?.enabled !== true🤖 Prompt for 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. In `@packages/start-plugin-core/src/post-build.ts` around lines 17 - 18, The variable spaOnly is inferred as boolean | undefined because startConfig.spa?.enabled can short-circuit to undefined; coerce the result to a true boolean so its type is boolean (e.g., wrap the whole expression with a boolean coercion such as Boolean(...) or double-negation) and keep the same logical check using startConfig.spa?.enabled and startConfig.prerender?.enabled !== true so usages like if (spaOnly) remain unchanged.
🤖 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 `@packages/start-plugin-core/tests/post-server-build.test.ts`:
- Around line 14-41: The test is misleading because postBuild does not read
globalThis.TSS_PRERENDER_DYNAMIC_ROUTES; remove the global setup and adjust the
test to assert the actual condition under test by deleting the
globalThis.TSS_PRERENDER_DYNAMIC_ROUTES assignment (and its cleanup) and rename
the it(...) description to something like "does not enable prerendering when
pages array is empty and prerender config is absent" while keeping the same call
to postBuild, the mocked adapter.prerender and the expectation that prerender
was not called; alternatively move this scenario into an integration test that
invokes the real prerender implementation if you intend to assert behavior
around TSS_PRERENDER_DYNAMIC_ROUTES.
---
Nitpick comments:
In `@packages/start-plugin-core/src/post-build.ts`:
- Around line 17-18: The variable spaOnly is inferred as boolean | undefined
because startConfig.spa?.enabled can short-circuit to undefined; coerce the
result to a true boolean so its type is boolean (e.g., wrap the whole expression
with a boolean coercion such as Boolean(...) or double-negation) and keep the
same logical check using startConfig.spa?.enabled and
startConfig.prerender?.enabled !== true so usages like if (spaOnly) remain
unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3acc396d-cd42-4fb7-9e36-8cac7d3d593d
📒 Files selected for processing (4)
packages/start-plugin-core/src/post-build.tspackages/start-plugin-core/src/prerender.tspackages/start-plugin-core/src/vite/prerender.tspackages/start-plugin-core/tests/post-server-build.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/start-plugin-core/src/vite/prerender.ts
- packages/start-plugin-core/src/prerender.ts
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Nx Cloud has identified a flaky task in your failed CI:
Since the failure was identified as flaky, the solution is to rerun CI. Because this branch comes from a fork, it is not possible for us to push directly, but you can rerun by pushing an empty commit:
git commit --allow-empty -m "chore: trigger rerun"
git push
🎓 Learn more about Self-Healing CI on nx.dev
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds route-level
prerenderParamssupport for TanStack Start so dynamic routes can declare which params/search combinations should be prerendered at build time.This lets routes like
/posts/$postIdgenerate static output for known params without manually listing every final URL in the global prerender config. It also makes dynamic prerendering first-class for sitemap generation, sositemap.xmlcan include URLs produced from route params instead of only statically discovered routes.Features
prerenderParamsroute options through Start route module augmentation.prerenderParamscallbacks with abort signal support.paramsand optionalsearchvalues.sitemapmetadata for generated dynamic pages.sitemap.xmlentries for dynamic URLs produced byprerenderParams.priority,changefreq,lastmod, images, news, and alternate refs.sitemapandprerenderoverrides.prerender.prerenderParamsTimeoutfor build-time callback timeout control.prerender.separateRouteOptionsBundleto control whether prerender-only route options are built separately from the final server bundle.Bundle Behavior
prerenderParamsandsitemaproute options from client bundles.prerender.separateRouteOptionsBundle: falsewhen users prefer fewer build steps or when an adapter cannot support the extra environment.Coverage
ssr: false/ selective SSR fixtures.sitemap.xmlgeneration from static routes, route-level sitemap metadata, and dynamicprerenderParamsentries.Notes On Adapter Coverage
Cloudflare, Netlify, and Nitro builds were smoke-tested locally.
Bun was not covered locally in this pass. The PR keeps
separateRouteOptionsBundleconfigurable so deployment adapters can opt out of the extra prerender route-options environment if needed, and Nitro already does this by default for compatibility.Docs
prerenderParams.sitemap.xmlgeneration for dynamic prerendered URLs.prerenderParamsTimeout.separateRouteOptionsBundle, including the Nitro compatibility behavior.