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

Typegen generates files outside of .react-router when using routes imported from a library #12993

Open
wilcoxmd opened this issue Feb 10, 2025 · 7 comments
Assignees
Labels

Comments

@wilcoxmd
Copy link

wilcoxmd commented Feb 10, 2025

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:

import { teamARoutes } from 'team-a-routes'
import { teamBRoutes } from 'team-b-routes'

const router = createBrowserRouter([...teamARoutes, ...teamBRoutes])

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:

// apps/my-app/app/routes.ts
import { teamARoutes } from 'team-a-routes';
import { teamBRoutes } from 'team-b-routes'

export default [ 
  // Assuming a workspace structure with apps/ and libs/ as top level directories,
  // imported route configs end up looking something like 
  // { path: "my-path", file: "../../../libs/team-a-routes/routes/my-route.jsx" }
  ...teamARoutes, 
  ...teamBRoutes,
] 

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

System:
    OS: macOS 15.3
    CPU: (10) arm64 Apple M1 Max
    Memory: 283.39 MB / 64.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.5.1 - ~/.nvm/versions/node/v20.5.1/bin/node
    Yarn: 1.22.22 - ~/.nvm/versions/node/v20.5.1/bin/yarn
    npm: 9.8.0 - ~/.nvm/versions/node/v20.5.1/bin/npm
    pnpm: 9.12.1 - ~/.nvm/versions/node/v20.5.1/bin/pnpm
    Watchman: 2024.12.02.00 - /opt/homebrew/bin/watchman
  Browsers:
    Chrome: 132.0.6834.160
    Safari: 18.3

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:

  1. Not generate types for those routes, since libraries can be type checked separately.
    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,

  1. Generate library routes' typings into the .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:

  1. The library's route module type definitions are generated into apps/test-app/libs/route-library/+types/library-route.ts.
  2. This file is not located in the .react-router/types folder, and it also attempts to import the source file using import("../library-route.js"). ../library-route.js is not a valid path in the test-app project, and causes tsc typechecks to fail.
@wilcoxmd
Copy link
Author

wilcoxmd commented Feb 11, 2025

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!

@jrestall
Copy link
Contributor

Here's another similar reproduction, except this one uses Nx to automatically load the app's referenced package's routes.

https://stackblitz.com/github/jrestall/remix-nx-monorepo/tree/jrestall/react-router?file=apps%2Fmy-remix-app%2Fapp%2Froutes.ts

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.

@wilcoxmd
Copy link
Author

wilcoxmd commented Feb 12, 2025

@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 href and matches though. Presumably we could get by in the short term without cross-package links and matches being typesafe if necessary, but we'd certainly love to be able to take advantage of those upcoming features if possible. Maybe our libraries can also export types/helpers somehow as a workaround.

@wilcoxmd
Copy link
Author

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!

@jrestall
Copy link
Contributor

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.

Image

I switch the current Params type to a RouteParams interface so that each package can use declaration merging to add additional params. There was a conflict with react-router's existing Params type, hence the rename. Then I generate a +register.ts file for each package so that each shared package has its route params defined within it. Meaning the feature packages can be easily shared or even published as a self-contained unit. This is important as we share features amongst multiple clients and apps.

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 workspace:* devDependency of the other feature package and using typescript includes to pull in the other packages +register.ts definitions from the package's node_modules. Then the RouteParams will typecheck for both features routes.

  // /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

@chist3r
Copy link

chist3r commented Feb 23, 2025

@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 getTypesPath, maybe with a help of a new config key workspaceRoot we can have a sense of the paths and do the required replace

@wilcoxmd maybe a git ignore pattern can "somewhat" be a simple workaround to hide the generated unwanted types

@pcattori pcattori self-assigned this Feb 24, 2025
@wilcoxmd
Copy link
Author

wilcoxmd commented Feb 25, 2025

@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 node_modules/.react-router, or some other special location?

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:

  • A Marketing feature library creates a package of routes: /marketing/campaigns, /marketing/campaigns/:id
  • An Items feature library creates a pack of routes: /items, /items/:id. /items/:id wants to link to Lib A's /marketing/campaigns/:id route
  • A consuming application decides to mount the marketing package under /tools, meaning the final path for marketing is /tools/marketing/.... It mounts the items library at /catalog, so the final path for the items routes is /catalog/items/....

How does the link from /catalog/items/:id to /tools/marketing/campaigns/:id look? Maybe you could reference the route by id or an exported url helper in case the final path shifts a bit? In that event, it's not totally clear to me what would happen if there a param or something required by one of the main app's parent routes. Maybe as a starting place you could assume that libraries like this need to be mounted at the application root.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants