better support for Tanstack Router and React Router

[jprosevear]

Closes #5476 (comment)

This make require a lot of setup for testing, but generally speaking relative links in routers don't work because the useHref'ed value is not used for navigation.

Originally investigated in heroui-inc/heroui

✅ Pull Request Checklist:

• Included link to corresponding React Spectrum GitHub Issue.
• Added/updated unit tests and storybook for this change (for new code or code which already has tests).
• Filled out test instructions.
• Updated documentation (if it already exists for this component).
• Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

https://stackblitz.com/edit/wfdcu9f2-5zvxtaag?file=App.jsx has a comparison of the React Router link and RA link behavior for the relative link. Go to /blogs in the preview

It is also worth noting that the http:// link href is wrong as well, perhaps the href return value in linkProps should also be guarded with shouldClientNavigate somehow

🧢 Your Project:

https://www.rhino-project.org/ , https://www.heroui.com/ (I'm not affiliated with them, but they are a user of this lib where this first showed up)

[jprosevear]

routerLinkProps.href is the useHref value, so that relative links are computed correctly. For instance if you are under /blogs in React Router, useHref will compute "/blogs/1" if href="1"

[jprosevear]

An alternative might be to call it with e.currentTarget.href

[jprosevear]

The test failures make sense broadly in that its not expecting to use the full path from useHref for navigation.

expect(navigate).toHaveBeenCalledWith('/base/foo', {foo: 'bar'});

would be the fix

[jprosevear]

@devongovett i think you wrote the related code originally?

[snowystinger]

Thanks for the PR, you'll need to sign the CLA https://react-spectrum.adobe.com/contribute.html#contributor-license-agreement
Also, if you could update the tests so they pass, it'd help with the review.

Thanks again!

[jprosevear]

Thanks for the PR, you'll need to sign the CLA https://react-spectrum.adobe.com/contribute.html#contributor-license-agreement Also, if you could update the tests so they pass, it'd help with the review.

That's not a problem, the key question is if the updated behaviour is correct or is the current behaviour "as designed" or something.

Also how to potentially handle host based links like https://example.com which are quasi-broken even now afaict.

[jprosevear]

Ok, I went back to first creating stories that I believe demonstrate the problem for better discussion.

First, you can see that the href prop on the link is incorrect (useHref constructs ).

react-spectrum/packages/@react-aria/link/src/useLink.ts

Line 70 in a861ca4

linkProps: mergeProps(domProps, routerLinkProps, {

merges the useHref outcome into the props and that becomes the href prop on the link.

Second, the next_nested link has the correct href attribute because routerProps.href is merge, but the onclick handler uses the original href (props.href == "next_nested" at

react-spectrum/packages/@react-aria/link/src/useLink.ts

Line 93 in a861ca4

router.open(e.currentTarget, e, props.href, props.routerOptions);

) instead of the instead of the useHref value ("nested/next_nested")

So the correct behavior should be:

• if the navigation will be done by the router use routerProps.href as the path to router.open AND pass routerProps.href as href from useLink
• if the navigation will not be done by the router, pass props.href back as href from useLink

the trick is how to detect navigation will be done by the router onside the onClick handler since shouldClientNavigate requires the dom element (however, perhaps parsing props.href and seeing if location.origin exists would be enough?)

@snowystinger @devongovett

[jprosevear]

Another alternative would be to add an isExternal flag on the Link component

[devongovett]

This is a breaking change. Shouldn't the router be handling this in the navigate function? That should do the same resolution as useHref.

[jprosevear]

This is a breaking change. Shouldn't the router be handling this in the navigate function? That should do the same resolution as useHref.

Great question:

        <a
          href="blogs/14"
          onClick={(e) => {
            e.preventDefault();
            navigate('blogs/14');
          }}
        >
          direct nav link
        </a>

works fine, I'm not sure why it fails with react-aria.

[devongovett]

I think it might depend on where the RouterProvider is rendered in the tree. In the examples here, useNavigate() is called outside the Route component, so it won't know the nested route context. However, useHref is passed down and is called inside the link component, so it does know the nested route context. Hence the potential mismatch.

You could fix this by calling useNavigate and rendering RouterProvider inside each <Route>, i.e. inside your page component, instead of outside the whole router.

[jprosevear]

I think it might depend on where the RouterProvider is rendered in the tree. In the examples here, useNavigate() is called outside the Route component, so it won't know the nested route context. However, useHref is passed down and is called inside the link component, so it does know the nested route context. Hence the potential mismatch.

Yes, I think this is the core of the issue.

You could fix this by calling useNavigate and rendering RouterProvider inside each <Route>, i.e. inside your page component, instead of outside the whole router.

This does work, but seems awkward; will investigate some alternatives.

[jprosevear]

Some findings:

1. React Router checks for absolute (external) vs non-absolute hrefs and handles them differently; https://github.com/remix-run/react-router/blob/3b45394fc7bd7c4aa58b802fda2558093217baca/packages/react-router/lib/dom/lib.tsx#L577

2. Both Tanstack Router and React Router are looking to prefetch links , Tanstack has a React Aria guide for building custom links https://tanstack.com/router/latest/docs/framework/react/guide/custom-link

3. Tanstack Router requires explicitness around relative routes (need a "from" https://tanstack.com/router/v1/docs/framework/react/guide/navigation#everything-is-relative - and Aria doesn't have that)

4. Tanstack Router does not support external refs

One thought is to separate router links and non-router links in React Aria.

[jprosevear]

So the Tanstack Link component does actually look for "external"

  const type: 'internal' | 'external' = React.useMemo(() => {
    try {
      new URL(`${to}`)
      return 'external'
    } catch {}
    return 'internal'
  }, [to])

The buildLocation (https://tanstack.com/router/latest/docs/framework/react/api/router/RouterType#buildlocation-method) router function in Tanstack Router however needs more than just "to" in order to navigate (ie params for Dynamic links - https://tanstack.com/router/latest/docs/framework/react/guide/navigation#dynamic-links )

const link = (
  <Link
    to="/blog/post/$postId"
    params={{
      postId: 'my-first-blog-post',
    }}
  >
    Blog Post
  </Link>
)

this would roughly be in Typescript (note the change to Omit):

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

but also requires router.useHref in React Aria to pass the routerOptions into useHref as well (not just the original href as per useHref={(to) => router.buildLocation({ to }).href} - something like useHref={(to, routerOptions) => router.buildLocation({ to, ...routerOptions }).href}

Testing link: https://stackblitz.com/edit/github-e3kdjjca

[jprosevear]

This is an attempt to get the rest of the options through to useHref so that dynamic links work in Tanstack Router - https://tanstack.com/router/latest/docs/framework/react/guide/navigation#dynamic-links

[jprosevear]

The typing of this is not fully correct - build the buildLocation and navigate options do overlap quite a bit. Also this module extension breaks the types in Link.stories.tsx because its global - however this is useful to temporarily check type safety.

[jprosevear]

https://tanstack.com/router/latest/docs/framework/react/guide/type-utilities#type-checking-navigate-options-with-validatenavigateoptions - the problem is that I don't think we can properly do the generics here and make sure that href causes type safety for things like params but @schiller-manuel will know better than me.

[jprosevear]

This is the change so that $dynamic gets replaced by React Aria.

[jprosevear]

@devongovett updated approach, and possibly @schiller-manuel can help

[jprosevear]

Next step might be to test Tabs or other components that support hrefs?