fix(history): cleanUrlOnDispose also prevents write after dispose (#6305

)

In real situations, it seems like #5966 forgot to account for `write` being called through `onInternalStateChange`, which is deferred and thus called after the dispose of the middleware/router.

This PR solves that situation by ensuring the router never writes after dispose if this option is set to `false`.

ref: #6304

packages/instantsearch.js/src/lib/routers/__tests__/history-integration.test.ts

@@ -0,0 +1,140 @@
+/**
+ * @jest-environment jsdom
+ */
+
+import { createSearchClient } from '@instantsearch/mocks';
+import { wait } from '@instantsearch/testutils/wait';
+
+import { connectPagination, connectSearchBox } from '../../../connectors';
+import instantsearch from '../../../index.es';
+import { index } from '../../../widgets';
+import historyRouter from '../history';
+
+beforeEach(() => {
+ window.history.pushState({}, '', '/');
+});
+
+const writeDelay = 10;
+const writeWait = 10 * writeDelay;
+
+test('keeps url with cleanUrlOnDispose: false', async () => {
+ const router = historyRouter({ writeDelay, cleanUrlOnDispose: false });
+
+ const indexName = 'indexName';
+ const search = instantsearch({
+ indexName,
+ searchClient: createSearchClient(),
+ routing: {
+ router,
+ },
+ });
+
+ search.addWidgets([
+ connectPagination(() => {})({}),
+ index({ indexName }).addWidgets([connectSearchBox(() => {})({})]),
+ ]);
+
+ search.start();
+
+ expect(window.location.search).toBe('');
+
+ // on nested index
+ search.renderState[indexName].searchBox!.refine('test');
+ // on main index
+ search.renderState[indexName].pagination!.refine(39);
+
+ await wait(writeWait);
+
+ expect(window.location.search).toBe(
+ `?${encodeURI('indexName[page]=40&indexName[query]=test')}`
+ );
+
+ search.dispose();
+
+ await wait(writeWait);
+
+ // URL has not been cleaned
+ expect(window.location.search).toBe(
+ `?${encodeURI('indexName[page]=40&indexName[query]=test')}`
+ );
+});
+
+test('clears url with cleanUrlOnDispose: true', async () => {
+ const router = historyRouter({ writeDelay, cleanUrlOnDispose: true });
+
+ const indexName = 'indexName';
+ const search = instantsearch({
+ indexName,
+ searchClient: createSearchClient(),
+ routing: {
+ router,
+ },
+ });
+
+ search.addWidgets([
+ connectPagination(() => {})({}),
+ index({ indexName }).addWidgets([connectSearchBox(() => {})({})]),
+ ]);
+
+ search.start();
+
+ expect(window.location.search).toBe('');
+
+ // on nested index
+ search.renderState[indexName].searchBox!.refine('test');
+ // on main index
+ search.renderState[indexName].pagination!.refine(39);
+
+ await wait(writeWait);
+
+ expect(window.location.search).toBe(
+ `?${encodeURI('indexName[page]=40&indexName[query]=test')}`
+ );
+
+ search.dispose();
+
+ await wait(writeWait);
+
+ // URL has been cleaned
+ expect(window.location.search).toBe('');
+});
+
+test('clears url with cleanUrlOnDispose: undefined', async () => {
+ const router = historyRouter({ writeDelay });
+
+ const indexName = 'indexName';
+ const search = instantsearch({
+ indexName,
+ searchClient: createSearchClient(),
+ routing: {
+ router,
+ },
+ });
+
+ search.addWidgets([
+ connectPagination(() => {})({}),
+ index({ indexName }).addWidgets([connectSearchBox(() => {})({})]),
+ ]);
+
+ search.start();
+
+ expect(window.location.search).toBe('');
+
+ // on nested index
+ search.renderState[indexName].searchBox!.refine('test');
+ // on main index
+ search.renderState[indexName].pagination!.refine(39);
+
+ await wait(writeWait);
+
+ expect(window.location.search).toBe(
+ `?${encodeURI('indexName[page]=40&indexName[query]=test')}`
+ );
+
+ search.dispose();
+
+ await wait(writeWait);
+
+ // URL has been cleaned
+ expect(window.location.search).toBe('');
+});

packages/instantsearch.js/src/lib/routers/history.ts

@@ -283,6 +283,11 @@ Please make sure it returns an absolute URL to avoid issues, e.g: \`https://algo
 
  private shouldWrite(url: string): boolean {
  return safelyRunOnBrowser(({ window }) => {
+ // When disposed and the cleanUrlOnDispose is set to false, we do not want to write the URL.
+ if (this.isDisposed && !this._cleanUrlOnDispose) {
+ return false;
+ }
+
  // We do want to `pushState` if:
  // - the router is not disposed, IS.js needs to update the URL
  // OR