TanStack / router

🤖 Fully typesafe Router for React (and friends) w/ built-in caching, 1st class search-param APIs, client-side cache integration and isomorphic rendering.
https://tanstack.com/router
MIT License
7.27k stars 495 forks source link

Route navigated from receives search parameters of the route navigated to #341

Closed louis-young closed 1 year ago

louis-young commented 1 year ago

Describe the bug

Firstly, thanks for building and maintaining awesome open-source libraries 🙂

I'll preface this by saying that I think this is probably a bug but it's possible that it's the intended behaviour.

When you navigate away from a route, the element that the route renders receives the search parameters (via useSearch) of the route that you're navigating to. This happens very quickly before the element of the route that you're navigating away from is unmounted and replaced with the element of the route that you navigated to. However, this is still problematic in my opinion.

In my situation, I'm happy to assume that search parameters will always be present if the application is used in the way it's intended to be used. This means I can "safely" use the search parameters on the page without worrying about them being possibly undefined. It seems odd that a route that is being navigated away from receives the search parameters for the route that is being navigated to momentarily before it's unmounted and the rendered route is changed.

Your Example Website or App

https://codesandbox.io/s/github/louis-young/react-location-issue?file=/README.md

Steps to Reproduce the Bug or Issue

  1. From Page A (the homepage), click the link to Go to Page B with search parameters.
  2. Open the console and look at the search parameter being logged from Page B.
  3. Click the navigation link to Page A and notice how the search parameters are updated and logged as undefined in Page B immediately before the new route element is rendered.

Expected behavior

I'd expect a route to not receive the search parameters of the route that is replacing it. I'd expect useSearch to not update the search parameters until the route has been replaced.

Screenshots or Videos

No response

Platform

Additional context

I've never had this issue with React Router and was surprised to find it after migrating a considerable chunk of a codebase. I'd love to find out of this is a bug or if it's the intended behaviour. I can see arguments for both sides because immediately after clicking a navigation link, the URL is updated and therefore so are the search parameters. However, I think the fact that there's a slight delay between the URL being updated and the new route being rendered should be taken into account when returning search parameters.

I looked at rolling a useDeferredSearch hook that would essentially cache the search parameters whilst the router was in a transitional state/replacing the current rendered route element but I couldn't find anything being exposed from any of the APIs to enable me to do this. It also seemed like a hack so I'd be interested to hear your thoughts.

Thanks again 🙂

tannerlinsley commented 1 year ago

I haven’t had time to dig into this on react location. But TanStack Router, the next version of this library, should fix this. Would you be willing to try an alpha release?

Tanner Linsley On Oct 19, 2022 at 3:41 PM +0100, Louis Young @.***>, wrote:

Describe the bug Firstly, thanks for building and maintaining awesome open-source libraries 🙂 I'll preface this by saying that I think this is probably a bug but it's possible that it's the intended behaviour. When you navigate away from a route, the element that the route renders receives the search parameters (via useSearch) of the route that you're navigating to. This happens very quickly before the element of the route that you're navigating away from is unmounted and replaced with the element of the route that you navigated to. However, this is still problematic in my opinion. In my situation, I'm happy to assume that search parameters will always be present if the application is used in the way it's intended to be used. This means I can "safely" use the search parameters on the page without worrying about them being possibly undefined. It seems odd that a route that is being navigated away from receives the search parameters for the route that is being navigated to momentarily before it's unmounted and the rendered route is changed. Your Example Website or App https://codesandbox.io/s/github/louis-young/react-location-issue?file=/README.md Steps to Reproduce the Bug or Issue

  1. From Page A (the homepage), click the link to Go to Page B with search parameters.
  2. Open the console and look at the search parameter being logged from Page B.
  3. Click the navigation link to Page A and notice how the search parameters are updated and logged as undefined in Page B immediately before the new route element is rendered.

Expected behavior I'd expect a route to not receive the search parameters of the route that is replacing it. I'd expect useSearch to not update the search parameters until the route has been replaced. Screenshots or Videos No response Platform

• OS: macOS and Windows • Browser: Chrome • Version: 105.0.5195.125 (Official Build) (arm64)

Additional context I've never had this issue with React Router and was surprised to find it after migrating a considerable chunk of a codebase. I'd love to find out of this is a bug or if it's the intended behaviour. I can see arguments for both sides because immediately after clicking a navigation link, the URL is updated and therefore so are the search parameters. However, I think the fact that there's a slight delay between the URL being updated and the new route being rendered should be taken into account when returning search parameters. I looked at rolling a useDeferredSearch hook that would essentially cache the search parameters whilst the router was in a transitional state/replacing the current rendered route element but I couldn't find anything being exposed from any of the APIs to enable me to do this. It also seemed like a hack so I'd be interested to hear your thoughts. Thanks again 🙂 — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: @.***>

louis-young commented 1 year ago

I haven’t had time to dig into this on react location. But TanStack Router, the next version of this library, should fix this. Would you be willing to try an alpha release? Tanner Linsley

I hadn't realised that the TanStack changes were different versions, I thought they were package name scope changes! I'd be more than happy to try an alpha release.

Thanks for the quick reply.

louis-young commented 1 year ago

How would I go about trying an alpha release? I can see some alpha releases on GitHub but they're pre-releases and there don't appear to be any alpha versions published on npm. If it's a case of waiting for one to be published then I'll keep an eye out 🙂

tannerlinsley commented 1 year ago

You can install it via @tanstack/react-router@alpha (core: @tanstack/router-core@alpha, devtools: @tanstack/react-router-devtools@alpha). The basic and kitchen-sink examples in the alpha branch are up to date with the latest alpha

sclydon commented 1 year ago

When transitioning to a new route, the component you are transitioning from renders a couple of more times with the updated router state. You can assert this by logging the pathname from the router state in the component that is being unmounted. It will display the pathname of the next route. I tried @tanstack/react-router@alpha and it has the same behaviour.

tannerlinsley commented 1 year ago

I’ll dig into this and ensure it doesn’t happen on the new version.

Tanner Linsley Sent with Spark On Oct 24, 2022 at 5:17 AM -0600, sclydon @.***>, wrote:

When transitioning to a new route, the component you are transitioning from renders a couple of more times with the updated router state. You can assert this by logging the pathname from the router state in the component that is being unmounted. It will display the pathname of the next route. I tried @@. and it has the same behaviour. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.>

louis-young commented 1 year ago

I've had some time to look at this with the alpha version and I can confirm that the issue is still reproducible. Thanks, @sclydon for taking a look.

I’ll dig into this and ensure it doesn’t happen on the new version.

Awesome, thanks. Do you have any predictions as to when the new version will be released, please? A ballpark date would be more than helpful. Thanks again.

tannerlinsley commented 1 year ago

Beta in 2 weeks, RC in a month release in 6 weeks.

Tanner Linsley Sent with Spark On Oct 25, 2022 at 6:09 AM -0700, Louis Young @.***>, wrote:

I've had some time to look at this with the alpha version and I can confirm that the issue is still reproducible. Thanks, @sclydon for taking a look.

I’ll dig into this and ensure it doesn’t happen on the new version. Awesome, thanks. Do you have any predictions as to when the new version will be released, please? A ballpark date would be more than helpful. Thanks again. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>

louis-young commented 1 year ago

Thank you, that’s great!

XpamAmAdEuS commented 1 year ago

Same issue with 0.0.1-alpha.10 Here example with zod confirm same error. Cannot reproduce same behavior as in my app. Sometimes instead of undefined get old sortColumn and zod confirm its wrong. ZodError: [ { "received": "title", "code": "invalid_enum_value", "options": [ "firstName", "email", "lastName", "activated", "langKey" ], "path": [ "usersView", "sortColumn" ], "message": "Invalid enum value. Expected 'firstName' | 'email' | 'lastName' | 'activated' | 'langKey', received 'title'" } ] at handleResult (index.mjs:480:23) at ZodObject.safeParse (index.mjs:591:16) at ZodObject.parse (index.mjs:571:29) at Object.validate (index.js:1428:86) at index.js:2012:18 at Array.map (<anonymous>) at Object.loadMatches (index.js:2010:45) at Object.loadLocation (index.js:1792:20) at index.js:1711:16 at index.js:762:22 https://codesandbox.io/s/react-router-search-issue-d7pmpb?file=/README.md

tannerlinsley commented 1 year ago

Pretty sure this is fixed in the latest version. Ping here if found otherwise.

vcarel commented 8 months ago

Hi, we are using react-location 3 in a pretty big application, and migrating to the v4 is a tedious operation. I don't have a clear idea how we are going to handle that.

In the meanwhile, this issue is pretty annoying because we use search params to manage tabs. When we leave a page having tabs, it is first rerendered on the wrong tab for like 500ms before. May I kindly ask you to reconsider fixing this issue ? asking as an early sponsor 🙏🙏🙏