algolia / react-instantsearch

⚡️ Lightning-fast search for React and React Native applications, by Algolia.
https://www.algolia.com/doc/guides/building-search-ui/what-is-instantsearch/react/
MIT License
1.97k stars 386 forks source link

Next.js SSR applied filters remains applied browsing a new page #3582

Closed matteorigotti closed 1 year ago

matteorigotti commented 2 years ago

🐛 Bug description

Updating react-instantsearch-hooks in our Next.js SSR project to version 6.30.2 from version 6.23.4 the applied refinements remains applied browsing between category pages. Is there a way to avoid this beahviour in the new version?

🔍 Bug reproduction

Steps to reproduce the behavior:

  1. Go to https://yd73ex.sse.codesandbox.io/
  2. Click on a link in the menu (i.e. Audio)
  3. Choose a filter from the right column (i.e. Price 49.99)
  4. Click on a different link in the menu (i.e. Appliances)
  5. The previous filter (eg. Price 49.99) remains applied

Live reproduction:

https://codesandbox.io/s/happy-night-yd73ex (forked from https://codesandbox.io/s/github/algolia/react-instantsearch/tree/v6.29.0/examples/hooks-next)

💭 Expected behavior

Our expectation is that when you change page (changing algolia query configuration) the applied filters are cleared according to the url routing

🖥 Screenshots

Environment

Additional context

Our way to filter algolia record between different category pages is using the Configure component: <Configure filters={`hierarchicalCategories.lvl0:"${category}"`} />

dhayab commented 2 years ago

Hi @matteorigotti , thanks for reporting this issue. There is indeed an issue when Next.js routing and InstantSearch routing work both at the same time. We still haven't found a suitable way to make them work together at the moment. This might explain why the refinements are not in sync anymore with the query parameters once they're removed by the route change.

As a workaround, you could reset the refinements when changing route by adding a components inside <InstantSearch> that will leverage the useClearRefinements() Hook.

Code example ```ts function ResetRefinements({ router }: { router: NextRouter }) { const { refine } = useClearRefinements(); useEffect(() => { const cb = (url: string) => { if (url !== router.asPath) { refine(); } }; router.events.on("routeChangeStart", cb); return () => router.events.off("routeChangeStart", cb); }, [router, refine]); return null; } ```

Note that this will generate an additional API request.

As a recommended alternative, you could make the category path part of InstantSearch routing by parsing the full URL in the history() router. You would not need the <Configure> widget anymore, but you'll probably need to stop relying on next/link for the menu links and instead change the category with useMenu() for example.

You can follow this guide for more information.

Note See https://github.com/algolia/react-instantsearch/issues/3582#issuecomment-1211721943 for an updated recommendation.

alan-artemest commented 2 years ago

Hi @dhayab , Thanks for your reply. Same issue here: this is something you’ll plan to fix? Just to know which is the best move for us. Both the workarounds you’ve suggested would impact our system either in terms of developing and testing efforts.

Thank you!

dhayab commented 2 years ago

Hi, we have in plan an overhaul of our approach to routing that hopefully will prevent conflicts with other third-party routers. We don't have an estimate of when it will be available though.

dominiksipowicz commented 2 years ago

next.js router and next/link provide tons of performance and SEO benefits out of the box. Category links and breadcrumb links on category pages ideally should be possible with using next.js components for those reasons.

dhayab commented 2 years ago

Currently, the state is left untouched between navigations that target the same dynamic route because React does not unmount the parent component.

In order to force React to remount <InstantSearch>, you can add a key prop set to the dynamic route query for example (category in your case).

<InstantSearch
  /* ... */
  key={category}
></InstantSearch>

You can find more info in Next.js documentation.

FabienMotte commented 1 year ago

@matteorigotti Did @dhayab's answer help you out?

We're doing a round of cleanup before migrating this repository to the new InstantSearch monorepo. This issue seems not to have generated much activity lately and is to be mostly solved, so we're going to close it.

matteorigotti commented 1 year ago

Yes, adding the key to InstantSearch component solves the issue. Thank you.