Skip to content
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

refactor(router-core): moving internals of Router to router core #3800

Merged
merged 13 commits into from
Mar 18, 2025

Conversation

birkskyum
Copy link
Member

@birkskyum birkskyum commented Mar 18, 2025

The router-core has a new RouterCore class, that the frameworks' Router class just extend

If there were framework specific methods on the Router, it would be trivial to overwrite those this way

This reduces the size of the adapter packages by a third (e.g. packages/react-router/src from ~7000 -> 4311 LOC)

Copy link

nx-cloud bot commented Mar 18, 2025

View your CI Pipeline Execution ↗ for commit 9487084.

Command Status Duration Result
nx affected --targets=test:eslint,test:unit,tes... ✅ Succeeded 5m 7s View ↗
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 2m 4s View ↗

☁️ Nx Cloud last updated this comment at 2025-03-18 18:05:22 UTC

Copy link

pkg-pr-new bot commented Mar 18, 2025

Open in Stackblitz

More templates

@tanstack/arktype-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/arktype-adapter@3800

@tanstack/create-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/create-router@3800

@tanstack/directive-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/directive-functions-plugin@3800

@tanstack/eslint-plugin-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/eslint-plugin-router@3800

@tanstack/create-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/create-start@3800

@tanstack/history

npm i https://pkg.pr.new/TanStack/router/@tanstack/history@3800

@tanstack/react-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router@3800

@tanstack/react-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-devtools@3800

@tanstack/react-router-with-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-with-query@3800

@tanstack/react-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start@3800

@tanstack/react-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-client@3800

@tanstack/react-start-config

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-config@3800

@tanstack/react-start-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-plugin@3800

@tanstack/react-start-router-manifest

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-router-manifest@3800

@tanstack/react-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-server@3800

@tanstack/router-cli

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-cli@3800

@tanstack/router-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-core@3800

@tanstack/router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools@3800

@tanstack/router-devtools-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools-core@3800

@tanstack/router-generator

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-generator@3800

@tanstack/router-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-plugin@3800

@tanstack/router-utils

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-utils@3800

@tanstack/router-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-vite-plugin@3800

@tanstack/server-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/server-functions-plugin@3800

@tanstack/solid-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router@3800

@tanstack/solid-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-devtools@3800

@tanstack/solid-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start@3800

@tanstack/solid-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-client@3800

@tanstack/solid-start-config

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-config@3800

@tanstack/solid-start-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-plugin@3800

@tanstack/solid-start-router-manifest

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-router-manifest@3800

@tanstack/solid-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-server@3800

@tanstack/start

npm i https://pkg.pr.new/TanStack/router/@tanstack/start@3800

@tanstack/start-api-routes

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-api-routes@3800

@tanstack/start-client-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-client-core@3800

@tanstack/start-config

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-config@3800

@tanstack/start-server-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-core@3800

@tanstack/start-server-functions-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-functions-client@3800

@tanstack/start-server-functions-fetcher

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-functions-fetcher@3800

@tanstack/start-server-functions-handler

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-functions-handler@3800

@tanstack/start-server-functions-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-functions-server@3800

@tanstack/start-server-functions-ssr

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-functions-ssr@3800

@tanstack/valibot-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/valibot-adapter@3800

@tanstack/virtual-file-routes

npm i https://pkg.pr.new/TanStack/router/@tanstack/virtual-file-routes@3800

@tanstack/zod-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/zod-adapter@3800

commit: 9487084

birkskyum added a commit that referenced this pull request Mar 18, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Similar to #3800 , this use base classes to contain shared methods of
the solid/react-router Route in the router-core package.
@birkskyum birkskyum force-pushed the move-most-of-router-to-router-core branch from 975bd31 to 1d8286c Compare March 18, 2025 12:07
@birkskyum birkskyum force-pushed the move-most-of-router-to-router-core branch from 1d8286c to efe78f0 Compare March 18, 2025 12:09
@birkskyum
Copy link
Member Author

There is a challenge here.

The Link component lose it's type inference, for instance on the onClick event of e2e/react-start/basic/src/components/DefaultCatchBoundary.tsx, because it can't tell any longer it's in a react context.

We have to explicit type it

@birkskyum
Copy link
Member Author

@chorobin , it's only types that are failing here. If I added the types in the user code e2e/example, then it works fine. would you be able to help debug how we can make the react-router Link have react types again?

@chorobin
Copy link
Contributor

chorobin commented Mar 18, 2025

@chorobin , it's only types that are failing here. If I added the types in the user code e2e/example, then it works fine. would you be able to help debug how we can make the react-router Link have react types again?

Unfortunately createRouter needs to return RouterCore in core. This is because RegisteredRouter checks for this, not the specific Router type. I know that Router in extends RouterCore and there is a way to get this work but at the cost of large structural comparisons causing some large regressions in TS performance. Specific framework methods need to be augmented into RouterCore currently to avoid this

@birkskyum birkskyum merged commit 27b0283 into main Mar 18, 2025
5 checks passed
@birkskyum birkskyum deleted the move-most-of-router-to-router-core branch March 18, 2025 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants