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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

NextJS router.query is stale after hitting back button #6002

Open
1 task done
avremel opened this issue Jan 16, 2024 · 7 comments
Open
1 task done

NextJS router.query is stale after hitting back button #6002

avremel opened this issue Jan 16, 2024 · 7 comments
Labels
triage Issues to be categorized by the team

Comments

@avremel
Copy link

avremel commented Jan 16, 2024

馃悰 Current behavior

If you hit a filter value, router.query gets populated correctly with the new URL. If you hit the browser back button, the facet selection gets cleared but router.query still has the old filter selection and the component doesn't even get re-rendered.

馃攳 Steps to reproduce

  1. Click on any Brand filter.
  2. In dev tools you will see router.query being logged with the brand filter.
  3. Hit the back button.
  4. In dev tools you won't see anything logged because router.query hasn't been updated.

Live reproduction

https://codesandbox.io/p/devbox/nextjs-routing-issue-x9p8gy

馃挱 Expected behavior

I expect:

  1. router.query to have a new value
  2. That new value to be an empty object
  3. The component to re-render and trigger a log of router.query

Package version

algoliasearch 4.14.3, react-instantsearch 7.5.1, NextJS 12.3.4

Operating system

Mac 11.6.5

Browser

Chrome 113.0.5672.126

Code of Conduct

  • I agree to follow this project's Code of Conduct
@avremel avremel added the triage Issues to be categorized by the team label Jan 16, 2024
@Haroenv
Copy link
Contributor

Haroenv commented Jan 16, 2024

Is this the same as #5987? If you patch InstantSearch to use history.pushState instead of the next router, does it work?

@avremel
Copy link
Author

avremel commented Jan 16, 2024

@Haroenv I'm using NextJS 12.3.4, which only has a page router.

How can I patch it to use history.pushState? The same as the Remix example in the docs?

@Haroenv
Copy link
Contributor

Haroenv commented Jan 16, 2024

Ah I misunderstood the problem, that's indeed not related. If you add a button unrelated to InstantSearch that pushes to the router like this (https://codesandbox.io/p/devbox/nextjs-routing-issue-forked-t7w35v?file=%2Fpages%2Findex.tsx%3A60%2C1-64%2C16):

      <button type="button" onClick={() => {
        router.push('?something=true', undefined, {shallow: true})
      }}>
        add to url
      </button>

It behaves the exact same. I believe because the push is shallow, next doesn't consider that the router and app component needs to re-render, but only what has listeners to the beforePopState router event.

What use case do you have for having the router being updated, does it cause downstream issues? I wonder if something can be said to the router in the beforePopState function (

) 脿 la "there was an update, please render root again", but not sure if that's possible. Maybe emitting a routeChangeComplete is possible, but I wouldn't bet on it.

I don't believe the router should be "not shallow", as then the backend gets fetched, which is definitely a waste of resources.

@avremel
Copy link
Author

avremel commented Jan 16, 2024

@Haroenv I'll give you an example from our app:

  1. Navigate to https://www.ajmadison.com/c/dryers/
  2. The title is Dyers
  3. Click on Fuel Type: Electric
  4. The tile Electric Dryers
  5. Hit the back button
  6. The title is still Electric Dryers

We use router.query to calculate the title, that's why we can't rely on a shallow update.
Same goes for which facets are shown on the page, they are dependent on the current route.

What would the alternative be? Should I be checking useInstantSearch instead?

Would it be possible add an option for shallow: false?

@Haroenv
Copy link
Contributor

Haroenv commented Jan 16, 2024

I'm not convinced shallow: false would even solve the issue if that's what's going on, however using useInstantSearch definitely will be updated with the ui at all times, so I recommend you use that for your source of truth instead of router. Does that work for you?

@avremel
Copy link
Author

avremel commented Jan 16, 2024

I will try using useInstantSearch. If I'm not mistaken, I had issues relying on it early on, which is why I resorted to router.query. I'll give it another try.

@Haroenv
Copy link
Contributor

Haroenv commented Jan 16, 2024

I guess an issue you may have is if it's inside InstantSearch, there may be a chance of a render where the component doesn't render (although I think that's just a client rendering potential problem, shouldn't happen when you server side render)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage Issues to be categorized by the team
Projects
None yet
Development

No branches or pull requests

2 participants