fix(router): skip router write on duplicate entries

src/lib/__tests__/RoutingManager-test.ts

@@ -313,12 +313,9 @@ describe('RoutingManager', () => {
 
       await runAllMicroTasks();
 
-      expect(router.write).toHaveBeenCalledTimes(2);
-      expect(router.write).toHaveBeenLastCalledWith({
-        indexName: {
-          query: 'Apple',
-        },
-      });
+      // The UI state hasn't changed so `router.write` wasn't called a second
+      // time
+      expect(router.write).toHaveBeenCalledTimes(1);
     });
 
     test('should keep the UI state up to date on first render', async () => {
@@ -363,12 +360,9 @@ describe('RoutingManager', () => {
 
       await runAllMicroTasks();
 
-      expect(router.write).toHaveBeenCalledTimes(2);
-      expect(router.write).toHaveBeenLastCalledWith({
-        indexName: {
-          query: 'Apple iPhone',
-        },
-      });
+      // The UI state hasn't changed so `router.write` wasn't called a second
+      // time
+      expect(router.write).toHaveBeenCalledTimes(1);
     });
 
     test('should keep the UI state up to date on router.update', async () => {
@@ -443,6 +437,84 @@ describe('RoutingManager', () => {
         },
       });
     });
+
+    test('skips duplicate route state entries', async () => {
+      let triggerChange = false;
+      const searchClient = createSearchClient();
+      const stateMapping = createFakeStateMapping({
+        stateToRoute(uiState) {
+          if (triggerChange) {
+            return {
+              ...uiState,
+              indexName: {
+                ...uiState.indexName,
+                triggerChange,
+              },
+            };
+          }
+
+          return uiState;
+        },
+      });
+      const history = createFakeHistory();
+      const router = createFakeRouter({
+        onUpdate(fn) {
+          history.subscribe(state => {
+            fn(state);
+          });
+        },
+        write: jest.fn(state => {
+          history.push(state);
+        }),
+      });
+
+      const search = instantsearch({
+        indexName: 'indexName',
+        searchClient,
+        routing: {
+          router,
+          stateMapping,
+        },
+      });
+
+      const fakeSearchBox: any = createFakeSearchBox();
+      const fakeHitsPerPage = createFakeHitsPerPage();
+
+      search.addWidgets([fakeSearchBox, fakeHitsPerPage]);
+
+      search.start();
+
+      await runAllMicroTasks();
+
+      // Trigger an update - push a change
+      fakeSearchBox.refine('Apple');
+
+      expect(router.write).toHaveBeenCalledTimes(1);
+      expect(router.write).toHaveBeenLastCalledWith({
+        indexName: {
+          query: 'Apple',
+        },
+      });
+
+      // Trigger change without UI state change
+      search.removeWidgets([fakeHitsPerPage]);
+
+      expect(router.write).toHaveBeenCalledTimes(1);
+
+      await runAllMicroTasks();
+
+      triggerChange = true;
+      // Trigger change without UI state change but with a route change
+      search.removeWidgets([fakeHitsPerPage]);
+
+      expect(router.write).toHaveBeenCalledTimes(2);
+      expect(router.write).toHaveBeenLastCalledWith({
+        indexName: {
+          query: 'Apple',
+          triggerChange: true,
+        },
+      });
+    });
   });
 
   describe('windowTitle', () => {

src/lib/routers/__tests__/history.test.ts

@@ -8,111 +8,6 @@ describe('life cycle', () => {
     jest.restoreAllMocks();
   });
 
-  it('does not write the same url twice', async () => {
-    const pushState = jest.spyOn(window.history, 'pushState');
-    const router = historyRouter({
-      writeDelay: 0,
-    });
-
-    router.write({ some: 'state' });
-    await wait(0);
-
-    router.write({ some: 'state' });
-    await wait(0);
-
-    router.write({ some: 'state' });
-    await wait(0);
-
-    expect(pushState).toHaveBeenCalledTimes(1);
-    expect(pushState.mock.calls).toMatchInlineSnapshot(`
-      Array [
-        Array [
-          Object {
-            "some": "state",
-          },
-          "",
-          "http://localhost/?some=state",
-        ],
-      ]
-    `);
-  });
-
-  it('does not write if already externally updated to desired URL', async () => {
-    const pushState = jest.spyOn(window.history, 'pushState');
-    const router = historyRouter({
-      writeDelay: 0,
-    });
-
-    const fakeState = { identifier: 'fake state' };
-
-    router.write({ some: 'state one' });
-
-    // external update before timeout passes
-    window.history.pushState(
-      fakeState,
-      '',
-      'http://localhost/?some=state%20two'
-    );
-
-    // this write isn't needed anymore
-    router.write({ some: 'state two' });
-    await wait(0);
-
-    expect(pushState).toHaveBeenCalledTimes(1);
-    expect(pushState.mock.calls).toMatchInlineSnapshot(`
-      Array [
-        Array [
-          Object {
-            "identifier": "fake state",
-          },
-          "",
-          "http://localhost/?some=state%20two",
-        ],
-      ]
-    `);
-
-    // proves that InstantSearch' write did not happen
-    expect(history.state).toBe(fakeState);
-  });
-
-  it('does not write the same url title twice', async () => {
-    const title = jest.spyOn(window.document, 'title', 'set');
-    const pushState = jest.spyOn(window.history, 'pushState');
-
-    const router = historyRouter({
-      writeDelay: 0,
-      windowTitle: state => `My Site - ${state.some}`,
-    });
-
-    expect(title).toHaveBeenCalledTimes(1);
-    expect(window.document.title).toBe('My Site - undefined');
-
-    router.write({ some: 'state' });
-    await wait(0);
-
-    router.write({ some: 'state' });
-    await wait(0);
-
-    router.write({ some: 'state' });
-    await wait(0);
-
-    expect(pushState).toHaveBeenCalledTimes(1);
-    expect(pushState.mock.calls).toMatchInlineSnapshot(`
-      Array [
-        Array [
-          Object {
-            "some": "state",
-          },
-          "My Site - state",
-          "http://localhost/?some=state",
-        ],
-      ]
-    `);
-
-    expect(title).toHaveBeenCalledTimes(2);
-    expect(window.document.title).toBe('My Site - state');
-  });
-
   it('writes after timeout is done', async () => {
     const pushState = jest.spyOn(window.history, 'pushState');
 

src/lib/routers/history.ts

@@ -128,11 +128,8 @@ class BrowserHistory implements Router {
     }
 
     this.writeTimer = window.setTimeout(() => {
-      if (window.location.href !== url) {
-        setWindowTitle(title);
-
-        window.history.pushState(routeState, title || '', url);
-      }
+      setWindowTitle(title);
+      window.history.pushState(routeState, title || '', url);
       this.writeTimer = undefined;
     }, this.writeDelay);
   }

src/middleware/createRouter.ts

@@ -2,7 +2,8 @@ import simpleStateMapping from '../lib/stateMappings/simple';
 import historyRouter from '../lib/routers/history';
 import { Index } from '../widgets/index/index';
 import { Middleware } from '.';
-import { Router, StateMapping, UiState } from '../types';
+import { Router, StateMapping, UiState, RouteState } from '../types';
+import { isEqual } from '../lib/utils';
 
 const walk = (current: Index, callback: (index: Index) => void) => {
   callback(current);
@@ -50,11 +51,19 @@ export const createRouter: RoutingManager = (props = {}) => {
       ...stateMapping.routeToState(router.read()),
     };
 
+    let lastRouteState: RouteState | undefined = undefined;
+
     return {
       onStateChange({ uiState }) {
-        const route = stateMapping.stateToRoute(uiState);
-
-        router.write(route);
+        const routeState = stateMapping.stateToRoute(uiState);
+
+        if (
+          lastRouteState === undefined ||
+          !isEqual(lastRouteState, routeState)
+        ) {
+          router.write(routeState);
+          lastRouteState = routeState;
+        }
       },
 
       subscribe() {