-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: prerendering #6256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: prerendering #6256
Conversation
📝 WalkthroughWalkthroughThis PR fixes prerendering failures in Nitro + SPA mode by refactoring the post-build logic into a separate plugin hook with proper ordering, updating the example's Nitro dependency to nightly, and enabling prerendering with route filtering in the example configuration. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
Comment |
|
View your CI Pipeline Execution ↗ for commit 8bdde46
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
examples/solid/start-basic-nitro/package.json (1)
20-20: Consider pinning to a specific nightly version for reproducibility.Using
@latestfor the nightly package means builds won't be reproducible and could break unexpectedly if a regression is introduced in a newer nightly. The PR description mentions a specific version (nitro-nightly v3.0.1-20251230-132916-74f2198d) that's known to work.🔎 Suggested change
- "nitro": "npm:nitro-nightly@latest", + "nitro": "npm:nitro-nightly@3.0.1-20251230-132916-74f2198d",Alternatively, if the intent is to always track the latest nightly for testing purposes, this is acceptable for an example project, but consider adding a comment explaining this choice.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
examples/solid/start-basic-nitro/package.jsonexamples/solid/start-basic-nitro/vite.config.tspackages/start-plugin-core/src/plugin.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety for all code
Files:
examples/solid/start-basic-nitro/vite.config.tspackages/start-plugin-core/src/plugin.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
examples/solid/start-basic-nitro/vite.config.tspackages/start-plugin-core/src/plugin.ts
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace protocol
workspace:*for internal dependencies in package.json files
Files:
examples/solid/start-basic-nitro/package.json
🧠 Learnings (3)
📚 Learning: 2025-11-02T16:16:24.898Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5732
File: packages/start-client-core/src/client/hydrateStart.ts:6-9
Timestamp: 2025-11-02T16:16:24.898Z
Learning: In packages/start-client-core/src/client/hydrateStart.ts, the `import/no-duplicates` ESLint disable is necessary for imports from `#tanstack-router-entry` and `#tanstack-start-entry` because both aliases resolve to the same placeholder file (`fake-start-entry.js`) in package.json during static analysis, even though they resolve to different files at runtime.
Applied to files:
examples/solid/start-basic-nitro/vite.config.tspackages/start-plugin-core/src/plugin.ts
📚 Learning: 2025-10-01T18:31:35.420Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: e2e/react-start/custom-basepath/src/routeTree.gen.ts:58-61
Timestamp: 2025-10-01T18:31:35.420Z
Learning: Do not review files named `routeTree.gen.ts` in TanStack Router repositories, as these are autogenerated files that should not be manually modified.
Applied to files:
examples/solid/start-basic-nitro/vite.config.ts
📚 Learning: 2025-12-25T13:04:55.492Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 6215
File: e2e/react-start/custom-basepath/package.json:13-17
Timestamp: 2025-12-25T13:04:55.492Z
Learning: In the TanStack Router repository, e2e test scripts are specifically designed to run in CI (which uses a Unix environment), so Unix-specific commands (like `rm -rf`, `&` for backgrounding, and direct environment variable assignments without `cross-env`) are acceptable in e2e test npm scripts.
Applied to files:
examples/solid/start-basic-nitro/vite.config.ts
🧬 Code graph analysis (1)
examples/solid/start-basic-nitro/vite.config.ts (1)
packages/router-core/src/route.ts (1)
path(1553-1555)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Preview
🔇 Additional comments (2)
examples/solid/start-basic-nitro/vite.config.ts (1)
18-28: LGTM! Configuration correctly enables prerendering with appropriate route filtering.The filter function properly excludes dynamic routes (
/users), non-existent routes, and deferred routes from prerendering. Disabling sitemap generation alongside prerendering makes sense to avoid sitemap entries for filtered routes.packages/start-plugin-core/src/plugin.ts (1)
347-361: Clean separation of post-build logic into a dedicated plugin.The architectural decision to extract the post-build hook into a separate plugin entry without
enforce: 'pre'is sound. This ensures proper ordering with other plugins that havebuildApphooks withorder: 'post', which resolves the prerendering issues with Nitro. The comment clearly documents the rationale.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/start-server-core/package.json (1)
82-82: Consider adding a tracking issue or inline comment for the temporary h3 downgrade.The version
h3@2.0.1-rc.5exists on npm with no known security vulnerabilities. However, since this is a temporary pin to work around a regression in h3 PR #1259, consider adding a TODO comment in the code or creating a tracking issue to upgrade h3 when the regression is fixed. The latest version is currently2.0.1-rc.7.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
packages/start-server-core/package.json
🧰 Additional context used
📓 Path-based instructions (1)
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace protocol
workspace:*for internal dependencies in package.json files
Files:
packages/start-server-core/package.json
🧠 Learnings (2)
📚 Learning: 2025-11-02T16:16:24.898Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5732
File: packages/start-client-core/src/client/hydrateStart.ts:6-9
Timestamp: 2025-11-02T16:16:24.898Z
Learning: In packages/start-client-core/src/client/hydrateStart.ts, the `import/no-duplicates` ESLint disable is necessary for imports from `#tanstack-router-entry` and `#tanstack-start-entry` because both aliases resolve to the same placeholder file (`fake-start-entry.js`) in package.json during static analysis, even though they resolve to different files at runtime.
Applied to files:
packages/start-server-core/package.json
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Applies to **/package.json : Use workspace protocol `workspace:*` for internal dependencies in package.json files
Applied to files:
packages/start-server-core/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Preview
89c0fd7 to
8bdde46
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
examples/solid/start-basic-nitro/package.json (1)
20-20: Consider pinning the nightly version for reproducibility.The PR description mentions a specific nightly version (
v3.0.1-20251230-132916-74f2198d), but@latestis used here, which will float to newer nightlies. This could cause reproducibility issues or unexpected breakage if future nightlies introduce changes.🔎 Suggested fix to pin the nightly version
- "nitro": "npm:nitro-nightly@latest", + "nitro": "npm:nitro-nightly@3.0.1-20251230-132916-74f2198d",
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
examples/solid/start-basic-nitro/package.jsonexamples/solid/start-basic-nitro/vite.config.tspackages/start-plugin-core/src/plugin.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/start-plugin-core/src/plugin.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace protocol
workspace:*for internal dependencies in package.json files
Files:
examples/solid/start-basic-nitro/package.json
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety for all code
Files:
examples/solid/start-basic-nitro/vite.config.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
examples/solid/start-basic-nitro/vite.config.ts
🧠 Learnings (3)
📚 Learning: 2025-11-02T16:16:24.898Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5732
File: packages/start-client-core/src/client/hydrateStart.ts:6-9
Timestamp: 2025-11-02T16:16:24.898Z
Learning: In packages/start-client-core/src/client/hydrateStart.ts, the `import/no-duplicates` ESLint disable is necessary for imports from `#tanstack-router-entry` and `#tanstack-start-entry` because both aliases resolve to the same placeholder file (`fake-start-entry.js`) in package.json during static analysis, even though they resolve to different files at runtime.
Applied to files:
examples/solid/start-basic-nitro/vite.config.ts
📚 Learning: 2025-10-01T18:31:35.420Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: e2e/react-start/custom-basepath/src/routeTree.gen.ts:58-61
Timestamp: 2025-10-01T18:31:35.420Z
Learning: Do not review files named `routeTree.gen.ts` in TanStack Router repositories, as these are autogenerated files that should not be manually modified.
Applied to files:
examples/solid/start-basic-nitro/vite.config.ts
📚 Learning: 2025-12-25T13:04:55.492Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 6215
File: e2e/react-start/custom-basepath/package.json:13-17
Timestamp: 2025-12-25T13:04:55.492Z
Learning: In the TanStack Router repository, e2e test scripts are specifically designed to run in CI (which uses a Unix environment), so Unix-specific commands (like `rm -rf`, `&` for backgrounding, and direct environment variable assignments without `cross-env`) are acceptable in e2e test npm scripts.
Applied to files:
examples/solid/start-basic-nitro/vite.config.ts
🧬 Code graph analysis (1)
examples/solid/start-basic-nitro/vite.config.ts (1)
packages/router-core/src/route.ts (1)
path(1553-1555)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Preview
🔇 Additional comments (1)
examples/solid/start-basic-nitro/vite.config.ts (1)
18-28: Prerender configuration looks good.The configuration correctly enables prerendering while disabling sitemap generation. The filter function appropriately excludes routes that shouldn't be prerendered (likely dynamic routes, non-existent test routes, and deferred content).
|
Thank you! Has this been cut for a release on |
after those two PRs are merged, this fixes prerendering with nitro:
nitrojs/nitro#3907
nitrojs/nitro#3908
needs https://www.npmjs.com/package/nitro-nightly/v/3.0.1-20251230-132916-74f2198d
closes #5967
close #5939
Summary by CodeRabbit
Chores
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.