-
-
Notifications
You must be signed in to change notification settings - Fork 10.5k
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
Typegen generates files outside of .react-router
when using routes imported from a library
#12993
Comments
I created a draft PR (#12996) that implements a fix assuming Expected Behavior 1 (not generating types for library routes) is desirable. Very open to discussion, feedback, or any other ideas! |
Here's another similar reproduction, except this one uses Nx to automatically load the app's referenced package's routes. Would be good to find a solution that supports typegen for routes in packages as with setups like Nx that's where most of the code is. |
@jrestall thanks for adding another reproduction! I agree too, it would be really nice to figure out how to support typegen for routes located in monorepo packages better. We're currently able to get by ok since our route packages also run their own react-router dev servers and typegen processes. I'm not sure exactly how things will look once we have better support for typesafe |
I opened up #12996 for review and further discussion. I do think it would be nice to have a way to import library types, but for now the goal of the PR and approach 1 above is to hopefully just prevent any unintended issues without changing the current typegen behavior too much. Happy to discuss more too! |
What about this approach for proper monorepo support? jrestall@651d0b4 For each route file, it will find the nearest package.json file and use that as the base directory for creating all typegen files for the routes in that package. ![]() I switch the current import "react-router";
declare module "react-router" {
interface Register {
params: RouteParams;
}
interface RouteParams {
"/transfers/:id": {
id: string;
};
}
} If cross-package strongly-typed href are needed, then I'm adding a // /packages/transfers/package.json
"devDependencies": {
"react-router": "workspace:*",
"accounts": "workspace:*"
} // tsconfig.json
"include": [
"**/*.ts",
"**/*.tsx",
"./.react-router/types/**/*",
"./node_modules/**/.react-router/types/*",
], cc: @pcattori |
@jrestall I think this would make the structure a bit opinionated making it hard to use app-libs monorepo structure, also locating the nearest package.json will not work when using a non-publishable library approach with no package.json Noticed the routes are generated in app's root directory with the same structure, so there's a bit of a hope there, it's just a matter of replacing the base app path with workspace path.. If I'm building up correctly it would be a matter of knowing the directories' paths in @wilcoxmd maybe a git ignore pattern can "somewhat" be a simple workaround to hide the generated unwanted types |
@chist3r yea, we could gitignore and TS ignore the extraneous files, or patch the package on our end to unblock ourselves. But, that's definitely not ideal and I'd love for react-router to account for the routes-as-libraries case too. It'll be really helpful for larger apps and the monorepo use case. @jrestall the idea of allowing extension of the Register and route params typings is really interesting! I think generating/replacing files in various node_modules packages feels like it could introduce some weirdness if that package isn't expecting it, but maybe that could be worked around by generating into I was thinking a bit more about the case of packaging a library of routes that you can publish and consume from another library. That case is more similar to our set up today. We've got libraries that run their own dev server, rather than relying on the host app, and are those are packaged and imported as npm packages into the final app's build. Something interesting to consider is that the library wouldn't have full control over final path it's mounted into, which could mean that any type or routing utils also need to account for not necessarily know the full path and all params ahead of time. Maybe there's a way to make the Register generic? 🤔 An possible example case to consider:
How does the link from |
I'm using React Router as a...
framework
Reproduction
Context
First, here's some context on why we're running into this issue:
We're migrating an application to React Router 7's new framework features. We've already migrated from v6 to v7, and our app is currently using createBrowserRouter to set up React Router on the client.
Our application routes are currently split up within a monorepo, and teams work on route groups in isolated projects. This allows us to easily split and cache parts of our builds, typechecks, tests, and other tasks across projects-- speeding up our CI and local dev experience. Groups of routes are imported into our main application as an npm package. For example:
We'd now like to migrate away from using
createBrowserRouter
to define our routes, and into a routes.ts file in order to start using more framework features. We'd like to import route definitions in a similar way that we used to in order to maintain our package structure and ability to split up project tasks. Something like:This works well with React Router at runtime. However, we've run into a an issue with the new typegen functionality when attempting this change. Specifically, route typings are generated outside the
.react-router
directory when we try to load our route libraries' files in the routes.ts file, which means the types files end up in our project code and cause type checking failures.Reproduction steps
You can find a reproduction repo here. The included README has steps to get set up and reproduce the issue.
System Info
Used Package Manager
pnpm
Expected Behavior
When provided route file paths are located in libraries, outside the current project's
appDirectory
, I would expect React Router to either:This seems like it could be a reasonable starting place to me, and resolves this issue well-enough in the short-term. We'd be happy to contribute this fix if it sounds good to you all!
or,
.react-router
folder.In the future, it could be nice for the main application to have access to imported route typings. However, this idea might require more thought on whether it can exist alongside the current generated types, or needs some special handling. This use case might also fit in somehow with the current work going on in draft: typesafety #12752? We'd be happy to help brainstorm further and contribute to this as well if helpful!
Actual Behavior
When provided route files that are located outside the project's
appDirectory
, the typegen command generates files outside of.react-router
. Those files also contain invalid path references when trying to import the source route module file.To illustrate, in the linked reproduction repo, you can observe that:
apps/test-app/libs/route-library/+types/library-route.ts.
import("../library-route.js")
.../library-route.js
is not a valid path in the test-app project, and causestsc
typechecks to fail.The text was updated successfully, but these errors were encountered: