-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix(solid-start): make Link active class reactive for include hash #6607
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
📝 WalkthroughWalkthroughThe PR refactors solid-router's link activation logic to use memoized currentLocation state with server-side rendering and hydration awareness, and enables a previously commented test assertion for hash-based link styling behavior. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit 71879ea
☁️ Nx Cloud last updated this comment at |
As part of #6416 and #6421 it was picked up that setting class active options was not functioning as expected when including hash during SSR rendering.
url hash fragments do not get sent to the server, this is only available on the client. so during SSR the url hash would not match next path and the active class options would not be applied. When the client is hydrating the isActive class, although now true, did not cause a rerender of the component to reflect the active status.
This was resolved in #6421 for React but not for Solid, Vue was working correctly.
This PR makes isActive reactive to the change in hydration status and applies the hash check correctly.
Summary by CodeRabbit
Bug Fixes
Tests