-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Description
Describe the bug
After clicking a link with a special character such as <Link to="/repro/$id" params={{ id: "✅" }} />, router's location state useRouterState({ select: s => s.location.pathname }) has non-encoded value /repro/✅.
This behavior might not be ideal since browser's actual location state is always encoded /repro/%E2%9C%85. For example, as illustrated in the reproduction below, simple browser reload will give an encoded pathname from useRouterState and this will make Link.activeProps to miss the original link before the reload.
Your Example Website or App
Steps to Reproduce the Bug or Issue
- Click "Repro 1" link
- Click browser reload button and see link active state changes
The same can be reproduced on stackblitz https://github.com/hi-ogawa/tanstack-router/tree/fix-pathname-encoding/examples/react/basic-ssr-file-based, but their reload button in IDE preview works differently, so you might need to use "Open preview in a new tab" and test there.
Expected behavior
I'm actually not sure what's the expected behavior, but it looks like react-router had a similar issue before. They seem to choose to always encode a pathname from their location state hook. Probably this is a relevant PR:
I haven't actually tested with react router, but what the author wrote in the PR description makes sense to me as an expected behavior.
Screenshots or Videos
2024-04-08.11-59-31.webm
Platform
- OS: Linux
- Browser: Chrome
- Version: 1.26.12 (main branch when this issue is created)
Additional context
On my react server framework https://github.com/hi-ogawa/vite-plugins/tree/main/packages/react-server, I'm using @tanstack/history for client side history management. I was trying to implement something like @tanstack/router's Link with activeProps in one of my demo hi-ogawa/react-server-demo-remix-tutorial#3. While doing that, I discovered this slight inconsistency and I found that the same issue can be reproduced on @tanstack/router, so I thought I'd report an issue here as well.
EDIT: My comment from here turns out to be irrelevant (see #1442 (comment)), so I'll hide it. The solution I ended up is to patch history.push/replace so that history only sees encoded version of href hi-ogawa/vite-plugins#276.
(open old comment)
Maybe the fix (if necessary) should be done in router integration and not in @tanstack/history itself since it provides createHref as an customization point. In my case, I plan to do something like this when doing createBrowserHistory:
const history = createBrowserHistory({
createHref(path) {
// consistently return encoded url as history state.
// i.e. history.push("/✅") should set { pathname: "/%E2%9C%85" } as state
const url = new URL(path, window.location.origin);
const newPath = url.href.slice(url.origin.length);
return newPath;
},
});But, I found that currently custom createHref doesn't reach history location object (note parseHref(destHref, ...) instead of parseHref(href, ...) below), so for now, I patched this part of the code. I wasn't sure if this is a bug of @tanstack/history, but if it looks like so, I'll probably open a separate issue for this later.
router/packages/history/src/index.ts
Lines 260 to 267 in f542349
| const href = createHref(destHref) | |
| if (!scheduled) { | |
| rollbackLocation = currentLocation | |
| } | |
| // Update the location in memory | |
| currentLocation = parseHref(destHref, state) |