Unwanted client-side query after SSR with Remix/Shopify-Hydrogen

[steve-jiang]

🐛 Current behavior

Using the latest Remix (or any version above 1.17.0) and Algolia's official SSR example for Remix, if we add a few more routes, navigating between the routes will trigger additional/unwanted client-side query. The page should be SSRed as the serverState is already in the loader, so the client-side query is not needed.

If you compare the queryId from the serverState with the client-side query, the queryIDs are different which will cause problems if ClickAnalytics is enabled.

After some research, we found that the issue is impacting all Remix apps that use 1.17.0 and above. Remix 1.17.0 uses the updated React-router that now supports React's Transaction API. For the transition-enabled router, it seems to cause unwanted fetches when the InstantSeach component hydrates after SSR

🔍 Steps to reproduce

1. Go to https://7lylvl-3000.csb.app/
2. Click the navigations on the top left corner
3. You should see both remix loader data and the Algolia query in the network tab in the dev tool.
4. The Algolia queries in below screenshot are unwanted

Live reproduction

https://codesandbox.io/p/sandbox/shy-leftpad-7lylvl

💭 Expected behavior

https://codesandbox.io/p/sandbox/broken-resonance-dwtqf2

Here is another minimum setup that uses Algolia's official Remix SSR example, but the remix version is 1.16.1 which doesn't uses the startTransition enabled router. If you click around the navigation, you can only see Remix's loader data fetches which contain the serverState that Algolia needs to render the page

https://dwtqf2-3000.csb.app/

Package version

"react-instantsearch": "7.0.2"

Operating system

macOs 13.4.1

Browser

Chrome 116.0.5845.140

Code of Conduct

• I agree to follow this project's Code of Conduct

[steve-jiang]

This issue remix-run/react-router#10579 shares some good insights about why some apps struggle with startTransition.

React-router 6.13.0 also provided a feature flag but recommended apps to adopt it sooner to better support concurrent mode. Unfortunately Remix opt-in to this flat automatically and doesn't provide the Remix users a feature flat to opt-out.

[aymeric-giraudet]

Hi @steve-jiang,

I think probably the only reason we want to run getServerState in the loader is to have the full markup for SEO. However, when navigating to another page, would it really make sense to run this request again server-side when the browser can hit Algolia APIs directly ? It probably is less performant, I see the loader takes at least 200ms to respond whereas the direct call to the Algolia API takes less than 80ms. I think startTransition really shines when you need the loader to read data from a database only the server can have access to for example.

I think what it does is that it's still the same <InstantSearch> component instance, and as it doesn't react to the initialResults props change, it just does the request by itself because it's reacting to the route change.

Is there a way to ignore running the loader when you change routes ? I know Next.js had a shallow prop in their <Link> component, not sure about Remix though.

[steve-jiang]

Hey @aymeric-giraudet,

I see where you're coming from, but I have some reservations. Your suggestion seems to diverge from Remix's design philosophy around the loader's functionality for data fetching.

Your point on performance stands valid only if the entire route solely fetches from Algolia, which isn't always the case. In e-commerce scenarios, we frequently fetch metadata from the e-commerce platform before or alongside Algolia's product fetch. Think about category pages: there's often a mix of content, some of which isn't even housed in Algolia. The beauty of Remix's loader, particularly with its defer and streaming capabilities, lies in its ability to smartly prioritise fetches based on content relevance. If we push Algolia's fetch to the client-side, it inevitably ends up being the last fetch of the waterfall. This is mainly because the client-side fetch is waiting for the loader to wrap up, and only then does <InstantSearch /> re-renders due to prop changes.

In a nutshell, moving the Algolia query to the client-side tacks on an extra network request since the loader's still got to grab non-Algolia content. With the loader, I can await the Algolia fetch and put the rest on defer, while client-side Algolia's request ends up at the end of the queue.

[aymeric-giraudet]

Hey @steve-jiang,

I've had another look by running it locally as CodeSandbox makes Remix have hydration problems (which I pointed out here codesandbox/codesandbox-client#7959)

Seems like even on initial request, the server gives the markup but we still send a request for nothing. And I was wrong about it being the same instance of <InstantSearch>, as they are wrapped in different components (ApplePage or SonyPage) so it does mount another instance, which is why it worked previously.

You made a great point about request waterfalls. Since we're currently working on Next.js app dir support, one thing we're about to resolve is to populate the algolia client cache with the initial results rather than changing the search function to a noop which seems unreliable now in both Next.js and Remix.
We should ship this in the coming 2 weeks, and I believe it should solve your issue as well.

I'll keep you updated when this is resolved !

[steve-jiang]

@aymeric-giraudet I saw you guys released a package for Next.js. Is that going to help Remix as well? Any suggestions to my original problem?

Thank you!

[aymeric-giraudet]

Hi @steve-jiang, it should be fixed whenever #5854 is merged.

[steve-jiang]

Hi @steve-jiang, it should be fixed whenever #5854 is merged.

@aymeric-giraudet can confirm the SSR works with the patch and no more duplicated Client-side queries. When do you reckon #5854 can be merged?

[Haroenv]

The code is solid, so you can use a patched version, but testing it proves not to be easy, so to keep correct expectations, I'd expect that patch to land in a couple weeks

[steve-jiang]

@Haroenv and @aymeric-giraudet now I tested on 7.3.0 and it is not working. I don't know what changed. Please see the minimum setup to replicate the issue:

Not working

Remix: 1.19.3
react-instantsearch: 7.3.0
https://codesandbox.io/p/sandbox/shy-leftpad-7lylvl?file=%2Fpackage.json%3A1%2C1
https://7lylvl-3000.csb.app/

Working

Remix: 1.16.1
react-instantsearch: 7.0.0
https://codesandbox.io/p/sandbox/broken-resonance-dwtqf2?file=%2Fpackage.json%3A24%2C35
https://dwtqf2-3000.csb.app/

[aymeric-giraudet]

Hi @steve-jiang,

Thanks a lot for the reproductions !

This is because all pages are using the same instance of searchClient, only the first initialResults will be cached.
This is an oversight on our end as we design the library around single page use, going from one product page to another in the InstantSearch model would normally just be done by calling refine rather than making an actual server request, in most use-cases.
There doesn't seem to be any problem with having a different searchClient per page, the same cache will be used across instances.

In your case this can be fixed by having each page have their own searchClient, like in this forked CodeSandbox :
https://codesandbox.io/p/sandbox/blue-wind-zt4gcp?file=%2Fapp%2Froutes%2Fsony.tsx%3A30%2C1-31%2C43

Note that it doesn't seem to work on CodeSandbox, but that's because there's a hydration error as I've mentioned before. It should work well locally.

[steve-jiang]

@aymeric-giraudet I downloaded your sandbox and ran locally and it still has the extra client-side query for each route change. Running it on local also uncovered another issue where if you refresh a search you see 2 duplicated client side queries.

The demo is done in a way where each route has its own InstantSearchSSR and InstantSearch instance, where normally you would have a dynamic route, e.g. /collections/$brand and each route will share the same InstantSearch component that will not get unmount -> mount during route change. In my previous experience, having different clients instances between dynamic routes would cause even more duplicated queries.

I made another reproduce now including cached and non-cached searchClient, as well as dynamic routes and running them in local all have client-side query when navigating, sometimes more duplicated ones for routes using new search client on render.

https://codesandbox.io/p/sandbox/happy-fire-2qrqwk?file=%2Fcomponents%2FNavs.tsx%3A18%2C27

Really keen to get this resolved and any help will be appraciated!

[aymeric-giraudet]

Ah yes sorry @steve-jiang,

I forgot that I had also removed insights={true} locally, which makes it work : https://zt4gcp-3000.csb.app/
https://codesandbox.io/p/sandbox/blue-wind-zt4gcp?file=%2Fapp%2Froutes%2Fapple.tsx%3A68%2C1

I think this is due to the generated anonymous insight token as it doesn't happen when you provide a user token yourself.

[aymeric-giraudet]

@steve-jiang

I fixed your examples there too (the ones where we initialize a new client) : https://x4lyps-3000.csb.app/
https://codesandbox.io/p/sandbox/strange-danilo-x4lyps?file=/components/SearchResults.tsx:95,5

Works with dynamic routes too. Here's what I did :

• removed insights={true} but you can also provide your own userToken to fix it
• Initialize the searchClient in useMemo, in your implem it was creating one for each render, whereas we want it to change per page
• Provide a key to InstantSearchSSRProvider to force a remount

I'll look into the insights and shared client issue on my end, but please tell me if you were able to fix it on your end 🙏

[steve-jiang]

@aymeric-giraudet Thank you so much for helping!

I can confirm it works if we use Algolia's demo index including latency and the same appId used in shopify-hydrogen-algolia.

However, it just doesn't work for our appId/Index and it took me a while to test but I still don't understand why. Is there a setting in the Algolia dashboard that we need to turn on to enable caching?

You can replicate it by downloading the codesandbox here: https://codesandbox.io/p/sandbox/happy-fire-2qrqwk?file=%2Fapp%2Falgolia.ts. I created a temp appkey for the codesandbox and if you can let me know when you done testing so I can remove it.

[Haroenv]

Good news, with the latest versions of InstantSearch and React InstantSearch this is solved: https://codesandbox.io/p/devbox/serene-wu-69wlhw?file=%2Fpackage.json%3A23%2C24 through #5946