-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: set progressiveChunkSize to infinity #5790
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
Conversation
WalkthroughThe SSR rendering stream configuration in React Router is updated to hardcode Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
|
View your CI Pipeline Execution ↗ for commit ff51c47
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/nitro-v2-vite-plugin
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-ssr-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-server
@tanstack/router-cli
@tanstack/router-core
@tanstack/router-devtools
@tanstack/router-devtools-core
@tanstack/router-generator
@tanstack/router-plugin
@tanstack/router-ssr-query-core
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-router-ssr-query
@tanstack/solid-start
@tanstack/solid-start-client
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-static-server-functions
@tanstack/start-storage-context
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
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/router-core/src/router.ts (1)
469-472: The React API is confirmed and properly typed—JSDoc documentation would be a nice addition.React 19's renderToReadableStream does support a progressiveChunkSize option (number of bytes per chunk), and your type annotation is correct. Consider adding JSDoc to document its purpose and default behavior for users:
ssr?: { nonce?: string + /** + * Controls the progressive chunk size threshold for SSR streaming. + * Number of bytes per chunk. Increase this value to reduce empty fallbacks + * during initial render. + */ progressiveChunkSize?: number }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/react-router/src/ssr/renderRouterToStream.tsx(2 hunks)packages/router-core/src/router.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/router-core/src/router.tspackages/react-router/src/ssr/renderRouterToStream.tsx
packages/router-core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep framework-agnostic core router logic in packages/router-core/
Files:
packages/router-core/src/router.ts
packages/{react-router,solid-router}/**
📄 CodeRabbit inference engine (AGENTS.md)
Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
Files:
packages/react-router/src/ssr/renderRouterToStream.tsx
⏰ 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)
packages/react-router/src/ssr/renderRouterToStream.tsx (1)
24-28: LGTM with one dependency.The implementation correctly forwards the
progressiveChunkSizeoption to both React rendering methods. The use of optional chaining ensures safe handling when the option is not provided. Both code paths (Web Streams and Node.js Streams) are consistently updated.However, this depends on verification that React's streaming APIs actually support this option (flagged in router.ts review).
Also applies to: 48-51
|
instead of making this a router option at all, i would just set the default to infinity and then expose a new cc @brhx |
|
agree - @andyabih please could you update your PR just to have the progressive chunk size set to Infinity for now? |
|
@brhx - Done! Happy to implement a PR with the proper options when you guys decide on the best way to tackle this. Pretty invested in this specifically due to SEO concerns, so always happy to help. |
fixes #5383
In reference to this: facebook/react@18212ca
Currently, when the shell hits ~12.5KB, any suspense boundary larger than 500 bytes triggers an empty fallback.
This results in pages loading without the content at first, and then having them render shortly after.
This PR simply adds a way to increase that size to whatever the user deems fit if he wishes to avoid this behavior.
Summary by CodeRabbit