fix: encoded url hash fragment matching #6416
Conversation
📝 WalkthroughWalkthroughAdds /specialChars/hash routes across React, Solid, and Vue examples, decodes URL hash in core parseLocation, guards hash-based active comparisons during SSR for Solid links, updates tests and prerender exclusions, and documents client-only hash fragment caveats. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser (Client)
participant RouterCore as Router Core
participant Server as Server (SSR)
participant SolidLink as Solid Link Component
Browser->>RouterCore: Request URL (includes raw `#fragment`)
RouterCore->>RouterCore: parseLocation -> decodePath(hash)
RouterCore-->>Browser: Location object with decoded hash
Note over Server,SolidLink: During SSR
Server->>SolidLink: render
SolidLink-->>Server: guard -> skip hash comparison (router.isServer)
Server-->>Browser: HTML without hash-dependent active state
Browser->>SolidLink: Hydrate / Navigate
SolidLink->>RouterCore: read location.hash (client)
SolidLink-->>Browser: compute active state using decoded hash
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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. Comment |
|
View your CI Pipeline Execution ↗ for commit 308cc5f
☁️ 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
🤖 Fix all issues with AI agents
In `@e2e/solid-start/basic/vite.config.ts`:
- Line 28: The prerender exclusion is using the wrong path string
'/search-params/hash' so the intended route '/specialChars/hash' will still be
prerendered; update the exclusion entry in the vite config (where the prerender
exclude array contains '/search-params/hash') to the correct route
'/specialChars/hash' so the hash route is properly skipped during prerendering.
🧹 Nitpick comments (2)
e2e/solid-start/basic/src/routes/specialChars/hash.tsx (1)
8-21: Consider simplifying the reactive hash access.The
createEffectto synclocation().hashto a signal adds indirection. Sincelocation()already returns a reactive accessor, you could potentially access the hash directly in JSX:function RouteComponent() { const location = useLocation() return ( <div data-testid="special-hash-heading"> Hello "/specialChars/hash"! <span data-testid="special-hash">{location().hash}</span> </div> ) }However, if the effect pattern is intentional for SSR/client synchronization (as mentioned in the PR objectives about hash checks being client-only), the current implementation is acceptable.
e2e/solid-start/basic/tests/special-characters.spec.ts (1)
123-128: Inconsistent assertion pattern for class check.This test uses
evaluate()withclassList.valueandtoContain(), while the router navigation test at lines 150-152 usestoHaveClass()directly. For consistency and better error messages, prefertoHaveClass():♻️ Proposed fix for consistency
- const el = await page - .getByTestId('special-hash-link') - .evaluate((e) => e.classList.value) - - await expect(el).toContain('font-bold') + await expect(page.getByTestId('special-hash-link')).toHaveClass( + 'font-bold', + )
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/router/framework/react/guide/navigation.md`:
- Around line 293-301: The doc text contains inconsistent casing for "URL" —
locate the paragraph that begins with "When directly navigating to a URL with a
hash fragment..." in navigation.md and change "Url" (or any non-URL casing) to
the uppercase "URL" everywhere in that paragraph and the surrounding bullet
examples so casing is consistent; ensure the sentence and examples read "URL"
(e.g., "the browser does not send the fragment to the server as part of the
request URL") and update any other instances in the same section.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@birkskyum when using hash fragments in url's these aren't available server side (the browser does not send this to the server), so during SSR the hash value is blank, but the value is provided once we are client side. However Solid does not treat this update in a reactive manner, i.e while the hash is updated, the location accessor does not notify subscribers to this change. This creates problems for cases when using the hash value i.e. Link active props, conditional rendering, etc. I've added a hacky workaround in the attached Solid e2e for this and made a change to the Link component to exclude the hash when checking if the hash should be used in the active props when server side, which solves the problem for Link active props. Ideally the location should update all the subscribers once the hash value is received, my knowledge of Solid is just too broken to comprehend where or what needs to change to do this the right way. Do you have any recommendations on what needs to change in solid-router for this or is this the best way to deal with this update? |
|
@nlynzaad I think this workaround to Link looks reasonable for solid v1. |
|
|
||
| if (local.activeOptions?.includeHash) { | ||
| // url hash is not available on server, so do not evaluate this here when on server | ||
| if (local.activeOptions?.includeHash && !router.isServer) { |
There was a problem hiding this comment.
This fix should be backported to the React package as well.
As there is the same hydratation issue in React, even after upgrading to TanStack Start v1.153.1.
There was a problem hiding this comment.
Thanks, could you create a reproducer in a new issue. i will look at it a bit later. We have tests for this specifically so would need to see what it is we missed.
This PR addresses some issues when matching url hashes.
As part of #6389 it was noticed that when using encoded characters in hashes these would not be matched correctly. This is addressed by making sure the encoded hash fragment is decoded before matching for Link active properties and others.
It was also noticed that in the case of solid hash matching for Link active properties was not working when doing SSR. This is due to how hashes is handled between server and client and compounded with how Solid's reactivity works. When a request is sent to the server during SSR the hash is removed. When received on client the hash is then processed but a rerender does not occur and hence not applying the active props. This is addressed by ensuring that the hash check is only done on client and not accounted for on server.
Tests have been added for the above
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.