Add an option to avoid router#write on InstantSearch unmount

[Haroenv]

Is your feature request related to a problem? Please describe 🙏

When InstantSearch is unmounted in Vue InstantSearch or Angular InstantSearch, a final write will happen, to bring the URL back to the default state without refinements. This is totally expected when the user isn't also using vue/angular router.

If they are using a router, there will be two entries in the history, e.g.:

?something=true (InstantSearch is mounted) ? (InstantSearch is unmounted) /other-url (next page)

Describe the solution you'd like 🤔

The final write in dispose of RoutingManager could become optional for cases like this:

in v3:

https://github.com/algolia/instantsearch.js/blob/7df31de4bb0337670ee25d9768a3c58600078978/src/lib/routers/history.ts#L191

in v4:

https://github.com/algolia/instantsearch.js/blob/03e64a0325e6cd469272bed0c7b48f47d4c6c28c/src/lib/routers/history.ts#L191

Describe alternatives you've considered ✨

An alternative would be to never write empty on unmount, but that would mean that when you simply unmount InstantSearch the URL would stay instead of becoming "non-InstantSearch".

Another alternative is making users use their own custom router, but this clearly is error-prone as it proves to be complicated.

Another alternative is not doing dispose on those cases, but that doesn't really make sense, since the event listener on pop state needs to be removed

Additional context

https://github.com/algolia/angular-instantsearch/issues/691 https://github.com/algolia/vue-instantsearch/issues/589

[sunscreem]

Use case for me on this issue:

I'm using the algolia history router to filter a load of products.

When the user is clicking on product (within ais-hits) I'm triggered url change vue-routers usual product-link.

At that point all the components start unmounting (as described above) and triggering more queries and unwanted url changes. (specifically - my product page url has now loaded but switched back to the filters url after about 500ms)

I've worked around it by forcing a complete url change window.location.href='/products/'+this.product which is ugly.

[dhayab]

This should be now handled thanks to https://github.com/algolia/instantsearch.js/pull/5045.

[Haroenv]

This is only fixed in some cases, but it's not possible to force never writing on dispose, which I think would solve some more SPA use cases. I'd like to keep this open I think

[dhayab]

Got it, thanks!

[ryanirilli]

Leaving a comment here for any React/Next.js users using react-instant-search and react-instantsearch-router-nextjs, we have done the following to bypass the query param cleanup on dispose

routing={{
          router: createInstantSearchRouterNext({
            singletonRouter,
            beforeDispose: () => {
              isDisposedRef.current = true
            },
            routerOptions: {
              push: (url: string) => {
                // We only want to allow InstantSearch to push if the component is not disposed.
                if (!isDisposedRef.current) {
                  router.push(url)
                }
              },
            },
          }),

[Haroenv]

Hmm that’s weird, normally those libraries already avoid writing on dispose. Do you have a repro of the issue?

[ryanirilli]

Hmm that’s weird, normally those libraries already avoid writing on dispose. Do you have a repro of the issue?

@Haroenv we're seeing the write happen in history.js here on dispose.

Our specific use case happens when shouldWrite returns true because latestAcknowledgedHistory does not agree with history.length this happens when using the back button, meaning the first navigation works fine but going back and then a subsequent navigation results in an extra push. In our code above we simply short-circuit the final push() when disposed.

[Haroenv]

Yes, but that goes into the push function which is then later stopped as the router is already disposed: https://github.com/algolia/instantsearch/blob/0820455ad291fb3e1910ebdfbcfa8d18eb811d31/packages/react-instantsearch-nextjs/src/InstantSearchNext.tsx#L86-L95

I do think in general that it's not a bad idea to add this.write({}) inside an option (default to true), called cleanup or similar. If you have that write removed, does everything work as expected? I'm happy to review a PR for it

[kevinv92]

Leaving a comment here for any React/Next.js users using react-instant-search and react-instantsearch-router-nextjs, we have done the following to bypass the query param cleanup on dispose

routing={{
          router: createInstantSearchRouterNext({
            singletonRouter,
            beforeDispose: () => {
              isDisposedRef.current = true
            },
            routerOptions: {
              push: (url: string) => {
                // We only want to allow InstantSearch to push if the component is not disposed.
                if (!isDisposedRef.current) {
                  router.push(url)
                }
              },
            },
          }),

Just encountered this issue. And this solution helped.

Encountered this issue when Next js does client side routing, when it loads the next page (clicking on item on result in the search page), it clears the query after a short time, and changes the URL. It pushes to the state so you have to navigate backwards twice to get back to the search page. Also we put tracking info as part of the query for the item's page and it was removing those.

[Haroenv]

forgot about this issue, it should have been fixed in https://github.com/algolia/instantsearch/pull/6305