fix(solid-start): fix hydration regression in 1.136.9 #5903
Conversation
WalkthroughThe Script component in the Solid router's Asset file now returns Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 ec9247a
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/solid-router/src/Asset.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
📚 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/solid-router/src/Asset.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: Preview
- GitHub Check: Test
🔇 Additional comments (1)
packages/solid-router/src/Asset.tsx (1)
128-128: Let me examine the Asset.tsx file directly to understand the full context of this change:Based on my research into Solid.js's isServer property, which is a non-reactive flag exported from solid-js/web that indicates whether code is executing on the server, I can now provide a definitive assessment:
The original review comment's concern about hydration mismatch appears to be based on a misunderstanding. When server and client render using isServer, they are separate reactive contexts, so the value change won't be detected on hydration. This means both server and client execute the same conditional logic—when the server evaluates
isServertotrueand returnsnull, the client will evaluateisServertofalsebut will execute the same logic path that also returnsnullduring hydration. Since both produce the same DOM output, there is no mismatch.The change to return
nullon line 128 (instead of an empty<script></script>) is correct because:
- It matches the server-side behavior when there is no
srcattribute and no stringchildren- Both client and server use identical conditional logic based on
isServer- The hydration context receives matching DOM from both sides
| }) | ||
|
|
||
| if (!router.isServer) { | ||
| // render an empty script on the client just to avoid hydration errors |
There was a problem hiding this comment.
Update the comment to match the new behavior.
The comment states "render an empty script on the client just to avoid hydration errors", but the code now returns null instead of an empty script tag.
Apply this diff to update the comment:
- // render an empty script on the client just to avoid hydration errors
+ // render nothing on the client to match server-side behavior and avoid hydration errors📝 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.
| // render an empty script on the client just to avoid hydration errors | |
| // render nothing on the client to match server-side behavior and avoid hydration errors |
🤖 Prompt for AI Agents
In packages/solid-router/src/Asset.tsx around line 127, update the inline
comment which currently reads "render an empty script on the client just to
avoid hydration errors" to reflect the new behavior: the component now returns
null on the client to avoid hydration errors; change the comment to something
like "return null on the client to avoid hydration errors" so it matches the
code.
birkskyum
deleted the
fix(solid-start)--fix-hydration-regression-in-1.136.9
branch
Fixes #5898
Summary by CodeRabbit