move hydrate call inside shared packages #5565
Closed
Conversation
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
Contributor
WalkthroughThe PR introduces a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:eslint,test:unit,tes... |
❌ Failed | 2m 15s | View ↗ |
nx run-many --target=build --exclude=examples/*... |
✅ Succeeded | 10s | View ↗ |
☁️ Nx Cloud last updated this comment at 2025-10-21 13:58:09 UTC
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: |
![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: 1
🧹 Nitpick comments (1)
packages/solid-start-client/src/StartClient.tsx (1)
10-14: Consider adding an explicit return type annotation.While TypeScript can infer
voidfor this function, adding an explicit return type would improve code clarity and align with strict type safety practices.Apply this diff to add the return type:
-export function clientHydrate() { +export function clientHydrate(): void { hydrateStart().then((router) => { hydrate(() => <StartClient router={router} />, document) }) }As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/solid-start-client/src/StartClient.tsx(1 hunks)packages/solid-start-client/src/index.tsx(1 hunks)packages/solid-start/src/default-entry/client.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/solid-start-client/src/index.tsxpackages/solid-start/src/default-entry/client.tsxpackages/solid-start-client/src/StartClient.tsx
packages/{*-start,start-*}/**
📄 CodeRabbit inference engine (AGENTS.md)
Name and place Start framework packages under packages/-start/ or packages/start-/
Files:
packages/solid-start/src/default-entry/client.tsx
🧬 Code graph analysis (2)
packages/solid-start/src/default-entry/client.tsx (1)
packages/solid-start-client/src/StartClient.tsx (1)
clientHydrate(10-14)
packages/solid-start-client/src/StartClient.tsx (2)
packages/react-start-client/src/StartClient.tsx (1)
StartClient(8-19)packages/solid-router/src/RouterProvider.tsx (1)
RouterProvider(44-53)
⏰ 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: Preview
- GitHub Check: Test
🔇 Additional comments (3)
packages/solid-start-client/src/index.tsx (1)
1-1: LGTM! Clean public API consolidation.The export statement correctly exposes both
StartClientand the newclientHydratefunction, providing a clean public API for the package.packages/solid-start/src/default-entry/client.tsx (1)
1-3: LGTM! Excellent simplification of the client entry point.The refactor successfully delegates the hydration logic to the shared
clientHydrate()function, making the default entry point minimal and maintainable. This aligns well with the PR objectives of centralizing hydration logic.packages/solid-start-client/src/StartClient.tsx (1)
2-3: LGTM! Correct imports for the hydration implementation.The imports of
hydrateStartandhydrateprovide the necessary functionality for client-side hydration.
Comment on lines
+10
to
+14
| export function clientHydrate() { | ||
| hydrateStart().then((router) => { | ||
| hydrate(() => <StartClient router={router} />, document) | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
Add error handling for the hydration promise.
The clientHydrate() function lacks error handling for the promise returned by hydrateStart(). If hydration fails, this will result in an unhandled promise rejection, which could silently break the application or cause confusing runtime errors.
Apply this diff to add error handling:
export function clientHydrate() {
- hydrateStart().then((router) => {
- hydrate(() => <StartClient router={router} />, document)
- })
+ hydrateStart()
+ .then((router) => {
+ hydrate(() => <StartClient router={router} />, document)
+ })
+ .catch((error) => {
+ console.error('Failed to hydrate application:', error)
+ throw 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.
Suggested change
| export function clientHydrate() { | |
| hydrateStart().then((router) => { | |
| hydrate(() => <StartClient router={router} />, document) | |
| }) | |
| } | |
| export function clientHydrate() { | |
| hydrateStart() | |
| .then((router) => { | |
| hydrate(() => <StartClient router={router} />, document) | |
| }) | |
| .catch((error) => { | |
| console.error('Failed to hydrate application:', error) | |
| throw error | |
| }) | |
| } |
🤖 Prompt for AI Agents
In packages/solid-start-client/src/StartClient.tsx around lines 10 to 14,
clientHydrate() currently calls hydrateStart().then(...) without catching
errors; add error handling to avoid unhandled promise rejections by appending a
.catch(...) to the promise chain (or use async/await with try/catch) that logs
the error (using console.error or an existing logger) and optionally handles
fallback behavior (e.g., abort hydration or show a user-facing error), ensuring
the catch provides clear context about the hydration failure.
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 by CodeRabbit