Replace react-router v5 with @tanstack/react-router #63
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| "@radix-ui/react-tabs": "^1.1.9", | ||
| "@tailwindcss/vite": "^4.1.5", | ||
| "@tanstack/react-router": "^1.119.0", | ||
| "@tanstack/react-router-devtools": "^1.119.1", |
There was a problem hiding this comment.
The existence of these devtools is pretty nifty
| /** | ||
| * forTesting indicates whether the app is for testing. | ||
| */ | ||
| forTesting?: boolean | undefined; |
There was a problem hiding this comment.
I realized that we never actually use this prop (grepped through the codebase) and there's probably a different way to set up the tanstack router for tests, so I got rid of it.
| const rootRoute = createRootRoute({ | ||
| component: () => ( | ||
| <> | ||
| <Outlet /> | ||
| <TanStackRouterDevtools /> | ||
| </> | ||
| ), | ||
| }); |
There was a problem hiding this comment.
You don't actually need a component in a rootRoute, but it seemed like a good place to put the dev tools, so I added an outlet and the dev tools here.
| const updated = datastore.update(currentItem!, value || ""); | ||
| if (updated && updated.pathname !== location.pathname) { | ||
| history.replace(updated.pathname); | ||
| navigate({ to: updated.pathname, replace: true }); |
There was a problem hiding this comment.
There's lots of this kind of change, because navigate behaves differently than the old history stuff did. I think they made that API change mostly to make navigate compatible with suspense and dataloading logic, which history's API couldn't really.
| const classes = useStyles(); | ||
| const history = useHistory(); | ||
| const navigate = useNavigate(); | ||
| const location = useLocation(); |
There was a problem hiding this comment.
Surprisingly, none of the location references needed to change.
| const indexRoute = createRoute({ | ||
| getParentRoute: () => rootRoute, | ||
| path: "$", | ||
| component: FullPlayground, | ||
| }); | ||
| const inlineRoute = createRoute({ | ||
| getParentRoute: () => rootRoute, | ||
| path: "/i/$", | ||
| component: InlinePlayground, | ||
| }); | ||
| const embeddedRoute = createRoute({ | ||
| getParentRoute: () => rootRoute, | ||
| path: "/e/$", | ||
| component: EmbeddedPlayground, | ||
| }); |
There was a problem hiding this comment.
This is how you define what they call "code-based routes" (as opposed to "file-based routes"). The docs strongly recommend file-based routing, but that would require more changes to the file structure of the application, so I chose not to do it in this PR. It may be something we want to do in the future.
| In | ||
| <Link className={classes.link} to={DataStorePaths.Schema()}> |
There was a problem hiding this comment.
Links work the same way and they don't make typescript angry anymore 🙌
| declare module "@tanstack/react-router" { | ||
| interface HistoryState { | ||
| range?: TextRange; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This is necessary to make typescript happy with us putting the textranges in state (type safety wooo)
| }); | ||
| const inlineRoute = createRoute({ | ||
| getParentRoute: () => rootRoute, | ||
| path: "/i/$", |
There was a problem hiding this comment.
I think the inline and embedded playgrounds should be working fine; I wasn't able to test them beyond hitting /i/schema and /e/schema and having it yell at me that it couldn't find the shared schema.
alecmerdler
left a comment
alecmerdler
left a comment
There was a problem hiding this comment.
Ran locally, seems to work. Nice job!
Description
I started working on replacing the tabs with ShadCN tabs, but I realized that the tabs really behave more like link-based routing elements than they do tabs that just show/hide some content. I started going to figure that out, but I ran into react-router v5 being a pain because of its lack of good types. Upgrading to RRv7 would have been a similar lift to changing to @tanstack/react-router, and this is a routing lib that I've wanted to try, and its API is both very type-safe and also similar enough to react-router's to make the migration pretty easy.
This is just the routing swap; I'll start developing on top of this in subsequent PRs to extend the usage of routes so that we're not doing as much
useLocation/useNavigatework.Changes
Will annotate
Testing
Review. Start this up in local dev and click around and see that everything still works as expected.