fix(nextjs): prevent onUpdate/onStateChange on own write (#5949)

* feat(nextjs): prevent onUpdate/onStateChange on own write Essentially, because we use router.push to write the URLs, we can't distinguish from a third-party URL/route update, which obviously should be taken in account for InstantSearch compared to a push we do ourselves in push/write, of which InstantSearch is already aware. Impact this would be preventing is double `onStateChange` calls when you're using next routing (CR-5006) not yet written a test, so not sure if this works, or if it's even the right approach (maybe url should be checked instead of an ad-hoc variable). As far as i can tell react-instantsearch-nextjs doesn't have this issue as it doesn't listen to the route change event (is that an oversight?) * wip tests (todo: find way to assert onStateChange without modifying example) * Update packages/react-instantsearch-router-nextjs/__tests__/e2e/onStateChange.test.ts * Update packages/react-instantsearch-router-nextjs/__tests__/e2e/onStateChange.test.ts * Update packages/react-instantsearch-router-nextjs/wdio.conf.cjs * fix tests * less test repetition * move onStateChange to dedicated test route --------- Co-authored-by: Aymeric Giraudet <aymeric.giraudet@algolia.com>
algolia · Dec 27, 2023 · bbb27fc · bbb27fc
1 parent 9d4a44c
commit bbb27fc
Show file tree

Hide file tree

Showing 5 changed files with 177 additions and 3 deletions.
diff --git a/examples/react/next-routing/pages/test.tsx b/examples/react/next-routing/pages/test.tsx
@@ -0,0 +1,84 @@
+// This is only to test `onStateChange` does not get called twice
+import algoliasearch from 'algoliasearch/lite';
+import { GetServerSideProps } from 'next';
+import Head from 'next/head';
+import Link from 'next/link';
+import singletonRouter from 'next/router';
+import React from 'react';
+import { renderToString } from 'react-dom/server';
+import {
+  InstantSearch,
+  RefinementList,
+  SearchBox,
+  InstantSearchServerState,
+  InstantSearchSSRProvider,
+  getServerState,
+} from 'react-instantsearch';
+import { createInstantSearchRouterNext } from 'react-instantsearch-router-nextjs';
+
+const client = algoliasearch('latency', '6be0576ff61c053d5f9a3225e2a90f76');
+
+type HomePageProps = {
+  serverState?: InstantSearchServerState;
+  url?: string;
+};
+
+export default function HomePage({ serverState, url }: HomePageProps) {
+  const [onStateChangeCalled, setOnStateChangeCalled] = React.useState(0);
+
+  return (
+    <InstantSearchSSRProvider {...serverState}>
+      <Head>
+        <title>React InstantSearch - Next.js</title>
+      </Head>
+
+      {/* If you have navigation links outside of InstantSearch */}
+      <Link href="/test?instant_search%5Bquery%5D=apple">Prefilled query</Link>
+
+      <InstantSearch
+        searchClient={client}
+        indexName="instant_search"
+        routing={{
+          router: createInstantSearchRouterNext({
+            singletonRouter,
+            serverUrl: url,
+            routerOptions: {
+              cleanUrlOnDispose: false,
+            },
+          }),
+        }}
+        insights={true}
+        onStateChange={({ uiState, setUiState }) => {
+          setOnStateChangeCalled((times) => times + 1);
+          setUiState(uiState);
+        }}
+      >
+        <output id="onStateChange">{onStateChangeCalled}</output>
+        <div className="Container">
+          <div>
+            <RefinementList attribute="brand" />
+          </div>
+          <div>
+            <SearchBox />
+          </div>
+        </div>
+      </InstantSearch>
+    </InstantSearchSSRProvider>
+  );
+}
+
+export const getServerSideProps: GetServerSideProps<HomePageProps> =
+  async function getServerSideProps({ req }) {
+    const protocol = req.headers.referer?.split('://')[0] || 'https';
+    const url = `${protocol}://${req.headers.host}${req.url}`;
+    const serverState = await getServerState(<HomePage url={url} />, {
+      renderToString,
+    });
+
+    return {
+      props: {
+        serverState,
+        url,
+      },
+    };
+  };
diff --git a/packages/react-instantsearch-router-nextjs/__tests__/e2e/nextLink.test.ts b/packages/react-instantsearch-router-nextjs/__tests__/e2e/nextLink.test.ts
@@ -10,7 +10,12 @@ describe('clicking on a Next.js link within the same page updates InstantSearch'
     await waitForUrl('http://localhost:3000/?instant_search%5Bquery%5D=apple');
 
     const searchInput = await $('.ais-SearchBox-input');
-    expect(await searchInput.getValue()).toBe('apple');
+
+    expect(
+      await browser.waitUntil(async () => {
+        return (await searchInput.getValue()) === 'apple';
+      })
+    ).toBe(true);
   });
 
   it('works when on a i18n route', async () => {
@@ -24,6 +29,10 @@ describe('clicking on a Next.js link within the same page updates InstantSearch'
     );
 
     const searchInput = await $('.ais-SearchBox-input');
-    expect(await searchInput.getValue()).toBe('apple');
+    expect(
+      await browser.waitUntil(async () => {
+        return (await searchInput.getValue()) === 'apple';
+      })
+    ).toBe(true);
   });
 });
diff --git a/packages/react-instantsearch-router-nextjs/__tests__/e2e/onStateChange.test.ts b/packages/react-instantsearch-router-nextjs/__tests__/e2e/onStateChange.test.ts
@@ -0,0 +1,73 @@
+import { waitForUrl } from './utils';
+
+describe('refining InstantSearch causes only one onStateChange', () => {
+  describe('Next.js Link', () => {
+    it('works when not on a i18n route', async () => {
+      await browser.url('/test');
+
+      const navigationLink = await $('a=Prefilled query');
+      await navigationLink.click();
+
+      await waitForUrl(
+        'http://localhost:3000/test?instant_search%5Bquery%5D=apple'
+      );
+
+      const searchInput = await $('.ais-SearchBox-input');
+      await browser.waitUntil(async () => {
+        return (await searchInput.getValue()) === 'apple';
+      });
+
+      const output = await $('output#onStateChange');
+      expect(await output.getText()).toBe('1');
+    });
+
+    it('works when on a i18n route', async () => {
+      await browser.url('/fr/test');
+
+      const navigationLink = await $('a=Prefilled query');
+      await navigationLink.click();
+
+      await waitForUrl(
+        'http://localhost:3000/fr/test?instant_search%5Bquery%5D=apple'
+      );
+
+      const searchInput = await $('.ais-SearchBox-input');
+      await browser.waitUntil(async () => {
+        return (await searchInput.getValue()) === 'apple';
+      });
+
+      const output = await $('output#onStateChange');
+      expect(await output.getText()).toBe('1');
+    });
+  });
+
+  describe('InstantSearch', () => {
+    it('works when not on a i18n route', async () => {
+      await browser.url('/test');
+
+      const refinementLink = await $('.ais-RefinementList-labelText=Apple');
+      await refinementLink.click();
+
+      await waitForUrl(
+        'http://localhost:3000/test?instant_search%5BrefinementList%5D%5Bbrand%5D%5B0%5D=Apple'
+      );
+
+      const output = await $('output#onStateChange');
+      expect(await output.getText()).toBe('1');
+    });
+
+    it('works when on a i18n route', async () => {
+      await browser.url('/fr/test');
+
+      const refinementLink = await $('.ais-RefinementList-labelText=Apple');
+      await refinementLink.click();
+
+      await waitForUrl(
+        'http://localhost:3000/fr/test?instant_search%5BrefinementList%5D%5Bbrand%5D%5B0%5D=Apple'
+      );
+
+      const output = await $('output#onStateChange');
+      expect(await output.getValue()).toBe('1');
+    });
+  });
+});
diff --git a/packages/react-instantsearch-router-nextjs/src/index.ts b/packages/react-instantsearch-router-nextjs/src/index.ts
@@ -91,6 +91,11 @@ export function createInstantSearchRouterNext<TRouteState = UiState>(
     });
   }
 
+  /**
+   * Marker to skip `routeChangeComplete` event when the push comes from the router itself.
+   */
+  let lastPushFromThis = false;
+
   const router: Router<TRouteState> & { _isNextRouter?: boolean } = history({
     start(onUpdate) {
       if (beforeStart) {
@@ -101,9 +106,10 @@ export function createInstantSearchRouterNext<TRouteState = UiState>(
       handler = () => {
         // Without this check, we would trigger an unnecessary search when navigating
         // to a page without InstantSearch
-        if (singletonRouter.pathname === initialPathname) {
+        if (singletonRouter.pathname === initialPathname && !lastPushFromThis) {
           onUpdate();
         }
+        lastPushFromThis = false;
       };
       singletonRouter.events.on('routeChangeComplete', handler);
 
@@ -161,6 +167,7 @@ export function createInstantSearchRouterNext<TRouteState = UiState>(
       singletonRouter.push(url, undefined, {
         shallow: true,
       });
+      lastPushFromThis = true;
     },
     ...routerOptions,
   });

diff --git a/packages/react-instantsearch-router-nextjs/wdio.conf.cjs b/packages/react-instantsearch-router-nextjs/wdio.conf.cjs
@@ -6,6 +6,7 @@ exports.config = {
   baseUrl: 'http://localhost:3000',
   waitforTimeout: 10000,
   services: ['selenium-standalone'],
+  seleniumInstallArgs: { drivers: { chrome: { version: '2.43' } } },
   capabilities: [
     {
       browserName: 'chrome',