fix(next-app-router): prevent client-side search when rerendering

[dhayab]

Summary

This PR updates <InstantSearchNext> so it only performs initial search in a server environment.

We previously relied on the availability of the initial results (injected in the document), but they are deleted once hydration is complete, which falsely triggers a client-side search when rerendering the component.

FX-3149

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 2ad0eb3:

Sandbox	Source
example-instantsearch-getting-started	Configuration
example-react-instantsearch-getting-started	Configuration
example-react-instantsearch-next-app-dir-example	Configuration
example-react-instantsearch-next-routing-example	Configuration
example-vue-instantsearch-getting-started	Configuration

[Haroenv]

won't this also be set if someone has InstantSearch set up with routing for multiple different path names?

[dhayab]

Not sure I get it, are you refering to a situation where there's an InstantSearch implementation in multiple paths and the user navigates from one to the other?

In that case, this is okay as search needs to be triggered.

In any case I'm not fond of having to store this data globally, but I haven't found a good way to track route changes since useRouter events are not available in app routing. I'm definitely open to ideas around that.

[Haroenv]

I'm thinking of a use case where with routing someone has for example /search/:category set up as the path. If you refine the category, won't it also be true here and force an extra search?

[dhayab]

This change caters to that use case specifically. You can see it in this comment https://github.com/algolia/instantsearch/pull/6452/files#r1863228512.

[aymeric-giraudet]

It would be nice if we had an InstantSearch singleton for React (that's what Apollo does iirc). It would make sure they only use one instance of InstantSearch and we could probably keep track of URLs in there.

[aymeric-giraudet]

InitializePromise and TriggerSearch wouldn't do anything on the client.

You don't need to have them to trigger a search on the client, normal CSR with 2 renders kicks in.

[aymeric-giraudet]

That means just having isServer is enough I think

[dhayab]

This breaks in a dynamic route setting, where a route change caused by Next.js will unmount/remount InstantSearchNext, and not trigger the search.

Adding the check for path change allows <TriggerSearch> to be mounted and executed as you can see in this recording (the generated sandbox is failing to setup sadly).

I don't exactly understand how it kicks in. Probably Next.js route change performs similarly to router.refresh() and executes server components?

screenshot-20241129-102623.mp4

[aymeric-giraudet]

Ah my bad sorry !
Actually I think this solution is nice, we could even choose not to delete initialResults and it would avoid that flash of no results. Not sure what the condition would look like though, but it can be done later in another PR.

[aymeric-giraudet]

And no I think it's because of something with InstantSearch RSC context that makes it not trigger the search even on the client.

[Haroenv]

as pathname doesn't seem to change when triggered by InstantSearch, this should work!

[oscarmylla]

Hi,

I’ve identified a bug related to this PR:

Bug Description:

When a user navigates away from a route containing InstantSearch, goes to a route without InstantSearch, and then uses the browser's back button to return, no server-side search is triggered. Additionally, because initialResults are deleted on unmount, no data is displayed. The path is still identified as the InstantSearchLastPath, leading to this issue.

Proposed Solution:

I suggest modifying the logic to ensure we correctly detect when the route has changed. Here's my suggestion that seem to work for me:

const hasRouteChanged = React.useMemo(() => {
    // On server, always return false
    if (isServer) return false;

    // On client, check if initial results are present.
    // If they are, the component has not unmounted and the route hasn't changed.
    const hasInitialResults = window[InstantSearchInitialResults] !== undefined;

    return !hasInitialResults;
  }, [isServer]);

[dhayab]

Hi @oscarmylla. Deducing the route change from the initial results availability is a better way to cover all use cases. I'll update the behavior in the next version of the package.

Thanks for your feedback!