oedotme / generouted

Generated file-based routes for Vite
https://stackblitz.com/github.com/oedotme/generouted/tree/main/explorer
MIT License
1.06k stars 50 forks source link

Feature: Customizable route id (react-router) #123

Closed jfairley closed 1 year ago

jfairley commented 1 year ago

react-router generates a route id for each route, but also exposes an attribute for a user-defined id. I would like to see this functionality exposed through generouted.

How is Route.id useful?

One reason Route.id is useful is for accessing loader data. react-router allows child nodes to fetch loader data from parent routes. There are a number of ways to achieve this, but the most straight-forward requires knowing the route id.

Example

To take advantage of useRouteLoaderData()

src/pages/(private)/_layout.tsx

export function Loader() {
  const user = /* fetch */;
  return user;
}

src/pages/(private)/some/custom/path.tsx

export default function Page() {
  const user = useRouteLoaderData("(private)@(private)/_layout")
  // ...
}

Clearly, this requires intimate knowledge of the full route tree.

Proposal

Support an optional well-known const for applying id to the generated route.

Example

export const ID = "load-user-data";

https://github.com/oedotme/generouted/blob/v1.15.8/packages/generouted/src/react-router.tsx#L21

- return { ...index, Component: Page, ErrorBoundary: module?.Catch, loader: module?.Loader, action: module?.Action }
+ return { ...index, Component: Page, ErrorBoundary: module?.Catch, loader: module?.Loader, action: module?.Action, id: module?.ID }

Alternatives

Aside from hard-coding the anticipated id as generated by react-router, there are a few current work-arounds, all of which are less palatable than declaring an ID.

  1. useMatches().find(({ data }) => /* data matches "user" */)?.data
  2. <Outlet context={user} /> / const user = useOutletContext()
    • My concern with this pattern is a middle-man layout changing the data unintentionally.
  3. custom react context
jfairley commented 1 year ago

Btw, I'm happy to try a go at a pull-request if you prefer it or are too busy.

oedotme commented 1 year ago

Hey @jfairley, thanks a lot for the proposal and the willing to contribute!

I was considering to have each route id matching it's full path without the extension which would be unique, then to make the current id value private for internal use. In the example you've provided, it would be (private)/_layout instead of (private)@(private)/_layout and so on.

This way you can directly guess what route to fill in based on the exact file path. On top of that, it can be generated as a type, providing auto-completion for the useRouteLoaderData hook similar to the useParams hook.

Also having the full path can be useful to be accessed in other cases, I found myself needing this couple of times before.

I believe having a type-safe useRouteLoaderData hook would make the experience easier without the need of guessing ids, but please let me know what you think.

jfairley commented 1 year ago

Hi @oedotme. Thanks for the response!

I also considered going with the partial file path as the id automatically, so I'm happy to hear you had considered that. I think that would be a great default regardless of whether we implement something like export const ID.

Lemme know if you want me to take a crack at that so as to not duplicate effort.

oedotme commented 1 year ago

Hey @jfairley, was a quick update to preserve the layout ids 081d1cc, src/pages/(private)/_layout.tsx should be now (private)/_layout instead of (private)@(private)/_layout which is a better default as you mentioned.

On the hand, I'll be adding a type-safe useRouteLoaderData hook as an enhancement in the future after some updates related to the current codegen process.

Thanks again for proposing to help, happy to collaborate with you more in the future!

jfairley commented 1 year ago

Hey @oedotme, I like the latest change (v.1.15.9). The generated paths are clear and predictable. 👍🏼

I look forward to type-safe useRouteLoaderData!


As far as issue-tracking goes, feel free to close this one unless you want it as a reminder for useRouteLoaderData.

oedotme commented 1 year ago

Hey @jfairley, yes we can close this issue — I'll make sure to leave a comment here tho once it's added. Thanks!