Skip to content

Search and path params in TanStack Router incompatible with useHref #6587

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

Open
levrik opened this issue Jun 20, 2024 · 21 comments
Open

Search and path params in TanStack Router incompatible with useHref #6587

levrik opened this issue Jun 20, 2024 · 21 comments

Comments

@levrik
Copy link

levrik commented Jun 20, 2024

Provide a general summary of the issue here

TanStack Router interpolates path params into the href:

<Link to="/projects/$id" params={{ id: '123' }}>Open</Link>

When using React Aria Components' router integration this gets turned into:

<Link href="/projects/$id" routerOptions={{ params: { id: '123' } }}>Open</Link>

As useHref of RouterProvider only receives the href but not routerOptions like navigate does, the navigation itself works correctly but the native features like Cmd+Click (or right-click "Open in new tab") navigate to the href template instead of the real link. Also the URL preview shown by the browser is wrong.

This could easily be solved by also passing routerOptions to useHref.

TanStack Router can also manage search params through a separate prop which suffers from the same issue.

I'm open to contributing this change but wanted to check upfront in case I'm missing something here. Maybe there is a reason for routerOptions not being passed to useHref that I'm unaware of.

🤔 Expected Behavior?

href on a tag to be /projects/123 with the above example.

😯 Current Behavior

href on a tag is /projects/$id with the above example.

💁 Possible Solution

Pass routerOptions to useHref as 2nd paramter.

🔦 Context

No response

@levrik
Copy link
Author

levrik commented Jun 21, 2024

I went ahead and created #6591 as I needed this change anyway to unblock myself at work. Let me know what you think.

@lithdew
Copy link
Contributor

lithdew commented Jun 21, 2024

What do you think about making href optional for components if routerOptions is provided?

For both Tanstack Router and React Router, routerOptions is solely needed in order to be able to generate the necessary href to attach to components. RouterProvider's useHref can then solely take in routerOptions instead of the href passed in to components.

@devongovett
Copy link
Member

You can also define the href type to accept an object instead of a string if you want.

declare module 'react-aria-components' {
  interface RouterConfig {
    href: YourRouterHref
  }
}

@levrik
Copy link
Author

levrik commented Jul 9, 2024

@devongovett Is there anything I can do to get a review on #6591? Maybe I missed something during the PR creation?

@devongovett
Copy link
Member

Sorry just busy and we had the week off last week for a holiday. Will discuss with the team.

@levrik
Copy link
Author

levrik commented Jul 10, 2024

@devongovett Thanks! Take your time.

@devongovett
Copy link
Member

devongovett commented Jul 12, 2024

We discussed this today. We were originally thinking that routerOptions was meant for things that affected the behavior of the navigation rather than only the URL itself (e.g. replace and resetScroll). We could add them to useHref but it would sort of mix concerns.

An alternative suggestion would be to define the href as an object corresponding to all of the options that TanStack router accepts rather than just a string.

import {ToOptions, NavigateOptions} from '@tanstack/react-router';

declare module 'react-aria-components' {
  interface RouterConfig {
    href: ToOptions;
    routerOptions: Omit<NavigateOptions, keyof ToOptions>;
  }
}

Then you can use them on a RAC component like this:

<RACLink
  href={{
    to: '/dashboard/invoices/$invoiceId',
    params: { invoiceId: 2 },
  }} />

and get autocomplete for both to and params.

This API also has the advantage that since both to and params are part of the same type, it's possible for typescript to infer the params based on the route. This would require a bit more work with TanStack Router's types to get working, but it should be possible. Also good news is that this should be possible today without any code changes.

Here's a StackBlitz example.

What do you think of this approach?

@levrik
Copy link
Author

levrik commented Jul 12, 2024

@devongovett That is an interesting idea. Let me try that in my codebase. Currently I'm using the patch from my PR and wrap components to fix the auto-completion but this sounds much cleaner.

@levrik
Copy link
Author

levrik commented Jul 12, 2024

@devongovett I gave it a try now but params are sadly not typed based on to but instead simply offer params from all routes. But I think that's what you meant by "This would require a bit more work with TanStack Router's types", right?

@devongovett
Copy link
Member

devongovett commented Jul 12, 2024

Yeah, that's what I meant. It should be possible, but I didn't quite figure it out. You'd basically need to generate a union of objects like {to: '/foo', params: {}} | {to: '/foo/$bar', params: {bar: string}}.

I tried something like this:

type Routes = NonNullable<ToOptions['to']>;
type Href = {
  [To in Routes]: ToOptions<RegisteredRouter, string, To>
}[Routes];

but it didn't seem to work. I'm probably missing something.

@evadecker
Copy link

evadecker commented Sep 2, 2024

Succeeded in getting params to work with a small modification building on @devongovett above. Here's what I changed from the recommendation in the docs:

import {type NavigateOptions, type ToOptions, useRouter} from '@tanstack/react-router';
import {RouterProvider} from 'react-aria-components';

declare module 'react-aria-components' {
  interface RouterConfig {
-   href: ToOptions['to'];
+   href: ToOptions;
    routerOptions: Omit<NavigateOptions, keyof ToOptions>;
  }
}

function RootRoute() {
  let router = useRouter();
  return (
    <RouterProvider
-     navigate={(to, options) => router.navigate({ to, ...options })}
+     navigate={(path, options) => router.navigate({ ...path, ...options })}
-     useHref={(to) => router.buildLocation(to).href}
+     useHref={({ to }) => router.buildLocation({ to }).href}
    >
      {/* ... */}
    </RouterProvider>
  );
}

This allows passing in params using an object for href:

<Link href={{ to: "/admin/forms/$formId", params: { formId: form._id } }} />

And still supports passing in a simple string for href. Spoke too soon, this approach requires refactoring all href="string" links to href={{ to: "string" }}. Although TypeScript is satisfied, the actual routing doesn't work, which I think is due to the use of useHref here and is also the reason for this issue: #6397

@levrik
Copy link
Author

levrik commented Sep 2, 2024

@evadecker You should be able to pass the whole object passed to useHref into buildLocation instead of just to.
I also tried this already but TBH I couldn't get params type being dependent on to to work, even when using ToOptions as href instead of just ToOptions["to"].

@evadecker
Copy link

@levrik Changing it to useHref={(path) => router.buildLocation(path).href} works the same as destructuring { to }. But passing a string as a component's href still throws an error:

CleanShot 2024-09-02 at 13 13 36@2x

@levrik
Copy link
Author

levrik commented Sep 2, 2024

@evadecker Passing as string doesn't work if you define just ToOptions as it's the whole interface and not just the to string.

@evadecker
Copy link

@levrik You're right. I could've sworn that ToOptions included | string but I must've been looking at TanStack's Link component. A bit confusing getting all the types to play nicely with React-Aria. I still haven't been able to get external links working properly.

@evadecker
Copy link

Any status update on #6591 or other changes here?

Unfortunately I'm still fighting with TanStack + RAC routing, and I'm a bit lost trying to dive into all the type definitions here between RAC's Link, RAC's RouterProvider, and TanStack, which overlap in ways that aren't totally clear to me.

It'd be great if RAC's Link component could handle these three cases out-of-the-box:

  • Accept an href as a string with Intellisense for existing routes (works today), e.g. <Link href="/about" />
  • Accept search and path params for dynamic routing with TanStack router (the changes suggested above enable this, but degrade DX by forcing engineers to provide an object to href instead of just a string), e.g. <Link href="/posts/$postId" params={{ postId: myPostId }} />
  • Accept an external link, e.g. <Link href="https://github.com" />

@mitchellwarr
Copy link

Just adding that this continues to be a problem and it would be good to have a solution or even a workaround

@mversic
Copy link

mversic commented Dec 3, 2024

First of all, big thanks to everyone in this thread.

This allows passing in params using an object for href:

<Link href={{ to: "/admin/forms/$formId", params: { formId: form._id } }} />

Passing entire ToOptions to useHref indeed works well. However, it doesn't work for relative paths. Consider this:

<Link href={{ from: "/admin/forms", to: "$formId", params: { formId: form._id } }} />

The compiler complains that to type is not valid

@levrik
Copy link
Author

levrik commented Dec 3, 2024

@mversic You have to type the full path in to. Only parameters that are already present in from are made optional in params. In your use-case there's no benefit of using from as there are no parameters in it. This is how TanStack Router works in general.

But I should also add that nailing down params doesn't work using ToOptions in RAC. params always accepts params of all routes, not only the one set in to. So while this works around the issue of useHref not receiving routerOptions, it doesn't fix the typing issues.

@mversic
Copy link

mversic commented Dec 3, 2024

@mversic You have to type the full path in to. Only parameters that are already present in from are made optional in params. In your use-case there's no benefit of using from as there are no parameters in it. This is how TanStack Router works in general.

thanks, I figured that parameters in from are optional. But still, I wanted to do this. That wouldn't work with RAC

But I should also add that nailing down params doesn't work using ToOptions in RAC. params always accepts params of all routes, not only the one set in to. So while this works around the issue of useHref not receiving routerOptions, it doesn't fix the typing issues.

oh, got it. Good that you pointed this out

@levrik
Copy link
Author

levrik commented Dec 3, 2024

But still, I wanted to do this. That wouldn't work with RAC

Yep as this requires properties to affect each other inside ToOptions which doesn't work within RAC. The same reason why params do not get nailed down based on to.

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

Successfully merging a pull request may close this issue.

6 participants