From e60ac95083252589d60543c09039c3944e37abe9 Mon Sep 17 00:00:00 2001 From: Dhaya <154633+dhayab@users.noreply.github.com> Date: Tue, 19 Dec 2023 11:28:32 +0100 Subject: [PATCH] feat(history): provide option to not clean url on dispose (#5966) --- bundlesize.config.json | 2 +- examples/js/e-commerce-umd/src/routing.ts | 1 + examples/js/e-commerce/src/routing.ts | 1 + examples/react/e-commerce/routing.ts | 1 + examples/react/next-routing/pages/index.tsx | 3 + examples/react/next/pages/index.tsx | 3 + examples/react/ssr/src/App.js | 1 + examples/vue/default-theme/src/App.vue | 4 +- examples/vue/e-commerce/src/routing.js | 1 + examples/vue/media/src/App.vue | 4 +- .../src/lib/routers/__tests__/history.test.ts | 138 ++++++++++++++++-- .../src/lib/routers/history.ts | 34 ++++- 12 files changed, 176 insertions(+), 17 deletions(-) diff --git a/bundlesize.config.json b/bundlesize.config.json index 1d3848926b..2f4543360c 100644 --- a/bundlesize.config.json +++ b/bundlesize.config.json @@ -14,7 +14,7 @@ }, { "path": "./packages/instantsearch.js/dist/instantsearch.development.js", - "maxSize": "167 kB" + "maxSize": "167.25 kB" }, { "path": "packages/react-instantsearch-core/dist/umd/ReactInstantSearchCore.min.js", diff --git a/examples/js/e-commerce-umd/src/routing.ts b/examples/js/e-commerce-umd/src/routing.ts index 16adfc8cdf..eadeb5dd02 100644 --- a/examples/js/e-commerce-umd/src/routing.ts +++ b/examples/js/e-commerce-umd/src/routing.ts @@ -80,6 +80,7 @@ function getCategoryName(slug: string): string { const originalWindowTitle = document.title; const router = window.instantsearch.routers.history({ + cleanUrlOnDispose: false, windowTitle({ category, query }) { const queryTitle = query ? `Results for "${query}"` : ''; diff --git a/examples/js/e-commerce/src/routing.ts b/examples/js/e-commerce/src/routing.ts index 4c96c685f0..5598133508 100644 --- a/examples/js/e-commerce/src/routing.ts +++ b/examples/js/e-commerce/src/routing.ts @@ -82,6 +82,7 @@ function getCategoryName(slug: string): string { const originalWindowTitle = document.title; const router = historyRouter({ + cleanUrlOnDispose: false, windowTitle({ category, query }) { const queryTitle = query ? `Results for "${query}"` : ''; diff --git a/examples/react/e-commerce/routing.ts b/examples/react/e-commerce/routing.ts index 2d2bb96351..0cd7e8e28f 100644 --- a/examples/react/e-commerce/routing.ts +++ b/examples/react/e-commerce/routing.ts @@ -76,6 +76,7 @@ function getCategoryName(slug: string): string { const originalWindowTitle = document.title; const router = historyRouter({ + cleanUrlOnDispose: false, windowTitle({ category, query }) { const queryTitle = query ? `Results for "${query}"` : ''; diff --git a/examples/react/next-routing/pages/index.tsx b/examples/react/next-routing/pages/index.tsx index ee50bc2a2e..c0239c7016 100644 --- a/examples/react/next-routing/pages/index.tsx +++ b/examples/react/next-routing/pages/index.tsx @@ -67,6 +67,9 @@ export default function HomePage({ serverState, url }: HomePageProps) { router: createInstantSearchRouterNext({ singletonRouter, serverUrl: url, + routerOptions: { + cleanUrlOnDispose: false, + }, }), }} insights={true} diff --git a/examples/react/next/pages/index.tsx b/examples/react/next/pages/index.tsx index f3f5340179..65cd36d6a9 100644 --- a/examples/react/next/pages/index.tsx +++ b/examples/react/next/pages/index.tsx @@ -57,6 +57,9 @@ export default function HomePage({ serverState, url }: HomePageProps) { router: createInstantSearchRouterNext({ serverUrl: url, singletonRouter, + routerOptions: { + cleanUrlOnDispose: false, + }, }), }} insights={true} diff --git a/examples/react/ssr/src/App.js b/examples/react/ssr/src/App.js index 5d7e479095..39cd1f77f7 100644 --- a/examples/react/ssr/src/App.js +++ b/examples/react/ssr/src/App.js @@ -33,6 +33,7 @@ function App({ serverState, location }) { routing={{ stateMapping: simple(), router: history({ + cleanUrlOnDispose: false, getLocation() { if (typeof window === 'undefined') { return location; diff --git a/examples/vue/default-theme/src/App.vue b/examples/vue/default-theme/src/App.vue index a77c1bd98e..165b15acfe 100644 --- a/examples/vue/default-theme/src/App.vue +++ b/examples/vue/default-theme/src/App.vue @@ -137,7 +137,9 @@ export default { '6be0576ff61c053d5f9a3225e2a90f76' ), routing: { - router: historyRouter(), + router: historyRouter({ + cleanUrlOnDispose: false, + }), stateMapping: simpleMapping(), }, }; diff --git a/examples/vue/e-commerce/src/routing.js b/examples/vue/e-commerce/src/routing.js index c24a51bb09..a52ce4330a 100644 --- a/examples/vue/e-commerce/src/routing.js +++ b/examples/vue/e-commerce/src/routing.js @@ -87,6 +87,7 @@ function getCategoryName(slug) { const originalWindowTitle = document.title; const router = historyRouter({ + cleanUrlOnDispose: false, windowTitle({ category, query }) { const queryTitle = query ? `Results for "${query}"` : ''; diff --git a/examples/vue/media/src/App.vue b/examples/vue/media/src/App.vue index 6a32d6a2b5..fd02f38d17 100644 --- a/examples/vue/media/src/App.vue +++ b/examples/vue/media/src/App.vue @@ -126,7 +126,9 @@ export default { '6be0576ff61c053d5f9a3225e2a90f76' ), routing: { - router: historyRouter(), + router: historyRouter({ + cleanUrlOnDispose: false, + }), stateMapping: simpleMapping(), }, }; diff --git a/packages/instantsearch.js/src/lib/routers/__tests__/history.test.ts b/packages/instantsearch.js/src/lib/routers/__tests__/history.test.ts index fac1ecdcb9..c53d24931e 100644 --- a/packages/instantsearch.js/src/lib/routers/__tests__/history.test.ts +++ b/packages/instantsearch.js/src/lib/routers/__tests__/history.test.ts @@ -28,7 +28,7 @@ describe('life cycle', () => { describe('pushState', () => { test('calls pushState on write', () => { const windowPushState = jest.spyOn(window.history, 'pushState'); - const router = historyRouter(); + const router = historyRouter({ cleanUrlOnDispose: true }); router.write({ indexName: { query: 'query' } }); jest.runAllTimers(); @@ -43,7 +43,7 @@ describe('life cycle', () => { test('debounces history push calls', () => { const windowPushState = jest.spyOn(window.history, 'pushState'); - const router = historyRouter(); + const router = historyRouter({ cleanUrlOnDispose: true }); router.write({ indexName: { query: 'query1' } }); router.write({ indexName: { query: 'query2' } }); @@ -61,7 +61,10 @@ describe('life cycle', () => { test('calls user-provided push if set', () => { const windowPushState = jest.spyOn(window.history, 'pushState'); const customPush = jest.fn(); - const router = historyRouter({ push: customPush }); + const router = historyRouter({ + push: customPush, + cleanUrlOnDispose: true, + }); router.write({ indexName: { query: 'query' } }); jest.runAllTimers(); @@ -83,6 +86,7 @@ describe('life cycle', () => { return 'Search'; }, getLocation, + cleanUrlOnDispose: true, }); expect(getLocation).toHaveBeenCalledTimes(1); @@ -90,7 +94,10 @@ describe('life cycle', () => { test('calls getLocation on read', () => { const getLocation = jest.fn(() => window.location); - const router = historyRouter({ getLocation }); + const router = historyRouter({ + getLocation, + cleanUrlOnDispose: true, + }); expect(getLocation).toHaveBeenCalledTimes(0); @@ -106,7 +113,10 @@ describe('life cycle', () => { test('calls getLocation on createURL', () => { const getLocation = jest.fn(() => window.location); - const router = historyRouter({ getLocation }); + const router = historyRouter({ + getLocation, + cleanUrlOnDispose: true, + }); router.createURL({ indexName: { query: 'query1' } }); @@ -117,7 +127,7 @@ describe('life cycle', () => { describe('pop state', () => { test('skips history push on browser back/forward actions', () => { const pushState = jest.spyOn(window.history, 'pushState'); - const router = historyRouter(); + const router = historyRouter({ cleanUrlOnDispose: true }); router.onUpdate((routeState) => { router.write(routeState); }); @@ -145,7 +155,7 @@ describe('life cycle', () => { }); test("doesn't throw if an index history state is null", () => { - const router = historyRouter(); + const router = historyRouter({ cleanUrlOnDispose: true }); const stateMapping = simple(); router.onUpdate((routeState) => { @@ -195,6 +205,7 @@ describe('life cycle', () => { search: '', } as unknown as Location; }, + cleanUrlOnDispose: true, }); const search = instantsearch({ indexName: 'indexName', @@ -226,6 +237,7 @@ describe('life cycle', () => { search: '', } as unknown as Location; }, + cleanUrlOnDispose: true, }); // We run the whole lifecycle to make sure none of the steps access `window`. @@ -243,7 +255,10 @@ describe('life cycle', () => { describe('onUpdate', () => { test('calls user-provided start function', () => { const start = jest.fn(); - const router = historyRouter({ start }); + const router = historyRouter({ + start, + cleanUrlOnDispose: true, + }); router.onUpdate(jest.fn()); @@ -254,19 +269,122 @@ describe('life cycle', () => { describe('dispose', () => { test('calls user-provided dispose function', () => { const dispose = jest.fn(); - const router = historyRouter({ dispose }); + const router = historyRouter({ + dispose, + cleanUrlOnDispose: true, + }); router.dispose(); expect(dispose).toHaveBeenCalledTimes(1); }); + + describe('cleanUrlOnDispose', () => { + const consoleSpy = jest.spyOn(global.console, 'info'); + consoleSpy.mockImplementation(() => {}); + + beforeEach(() => { + consoleSpy.mockReset(); + }); + + test('cleans refinements from URL if not defined', () => { + const windowPushState = jest.spyOn(window.history, 'pushState'); + const router = historyRouter(); + + expect(consoleSpy) + .toHaveBeenCalledWith(`Starting from the next major version, InstantSearch will not clean up the URL from active refinements when it is disposed. + +We recommend setting \`cleanUrlOnDispose\` to false to adopt this change today. +To stay with the current behaviour and remove this warning, set the option to true. + +See documentation: https://www.algolia.com/doc/api-reference/widgets/history-router/js/#widget-param-cleanurlondispose`); + + router.write({ indexName: { query: 'query1' } }); + jest.runAllTimers(); + + expect(windowPushState).toHaveBeenCalledTimes(1); + expect(windowPushState).toHaveBeenLastCalledWith( + { + indexName: { query: 'query1' }, + }, + '', + 'http://localhost/?indexName%5Bquery%5D=query1' + ); + + router.dispose(); + jest.runAllTimers(); + + expect(windowPushState).toHaveBeenCalledTimes(2); + expect(windowPushState).toHaveBeenLastCalledWith( + {}, + '', + 'http://localhost/' + ); + }); + + test('cleans refinements from URL if `true`', () => { + const windowPushState = jest.spyOn(window.history, 'pushState'); + const router = historyRouter({ cleanUrlOnDispose: true }); + + expect(consoleSpy).not.toHaveBeenCalled(); + + router.write({ indexName: { query: 'query1' } }); + jest.runAllTimers(); + + expect(windowPushState).toHaveBeenCalledTimes(1); + expect(windowPushState).toHaveBeenLastCalledWith( + { + indexName: { query: 'query1' }, + }, + '', + 'http://localhost/?indexName%5Bquery%5D=query1' + ); + + router.dispose(); + jest.runAllTimers(); + + expect(windowPushState).toHaveBeenCalledTimes(2); + expect(windowPushState).toHaveBeenLastCalledWith( + {}, + '', + 'http://localhost/' + ); + }); + + test('does not clean refinements from URL if `false`', () => { + const windowPushState = jest.spyOn(window.history, 'pushState'); + const router = historyRouter({ cleanUrlOnDispose: false }); + + expect(consoleSpy).not.toHaveBeenCalled(); + + router.write({ indexName: { query: 'query1' } }); + jest.runAllTimers(); + + expect(windowPushState).toHaveBeenCalledTimes(1); + expect(windowPushState).toHaveBeenLastCalledWith( + { + indexName: { query: 'query1' }, + }, + '', + 'http://localhost/?indexName%5Bquery%5D=query1' + ); + + router.dispose(); + jest.runAllTimers(); + + expect(windowPushState).toHaveBeenCalledTimes(1); + }); + }); }); describe('createURL', () => { test('prints a warning when created URL is not valid', () => { warning.cache = {}; - const router = historyRouter({ createURL: () => '/search' }); + const router = historyRouter({ + createURL: () => '/search', + cleanUrlOnDispose: true, + }); expect(() => router.createURL({ indexName: {} })) .toWarnDev(`[InstantSearch.js]: The URL returned by the \`createURL\` function is invalid. diff --git a/packages/instantsearch.js/src/lib/routers/history.ts b/packages/instantsearch.js/src/lib/routers/history.ts index 5d887ccb08..d659fcd226 100644 --- a/packages/instantsearch.js/src/lib/routers/history.ts +++ b/packages/instantsearch.js/src/lib/routers/history.ts @@ -1,6 +1,6 @@ import qs from 'qs'; -import { safelyRunOnBrowser, warning } from '../utils'; +import { createDocumentationLink, safelyRunOnBrowser, warning } from '../utils'; import type { Router, UiState } from '../../types'; @@ -27,6 +27,14 @@ export type BrowserHistoryArgs = { start?: (onUpdate: () => void) => void; dispose?: () => void; push?: (url: string) => void; + /** + * Whether the URL should be cleaned up when the router is disposed. + * This can be useful when closing a modal containing InstantSearch, to + * remove active refinements from the URL. + * @default true + */ + // @MAJOR: Switch the default to `false` and remove the console info in the next major version. + cleanUrlOnDispose?: boolean; }; const setWindowTitle = (title?: string): void => { @@ -97,10 +105,9 @@ class BrowserHistory implements Router { private latestAcknowledgedHistory: number = 0; private _start?: (onUpdate: () => void) => void; - private _dispose?: () => void; - private _push?: (url: string) => void; + private _cleanUrlOnDispose: boolean; /** * Initializes a new storage provider that syncs the search state to the URL @@ -115,6 +122,7 @@ class BrowserHistory implements Router { start, dispose, push, + cleanUrlOnDispose, }: BrowserHistoryArgs) { this.windowTitle = windowTitle; this.writeTimer = undefined; @@ -125,6 +133,20 @@ class BrowserHistory implements Router { this._start = start; this._dispose = dispose; this._push = push; + this._cleanUrlOnDispose = + typeof cleanUrlOnDispose === 'undefined' ? true : cleanUrlOnDispose; + + if (__DEV__ && typeof cleanUrlOnDispose === 'undefined') { + // eslint-disable-next-line no-console + console.info(`Starting from the next major version, InstantSearch will not clean up the URL from active refinements when it is disposed. + +We recommend setting \`cleanUrlOnDispose\` to false to adopt this change today. +To stay with the current behaviour and remove this warning, set the option to true. + +See documentation: ${createDocumentationLink({ + name: 'history-router', + })}#widget-param-cleanurlondispose`); + } safelyRunOnBrowser(({ window }) => { const title = this.windowTitle && this.windowTitle(this.read()); @@ -250,7 +272,9 @@ Please make sure it returns an absolute URL to avoid issues, e.g: \`https://algo clearTimeout(this.writeTimer); } - this.write({} as TRouteState); + if (this._cleanUrlOnDispose) { + this.write({} as TRouteState); + } } public start() { @@ -324,6 +348,7 @@ export default function historyRouter({ start, dispose, push, + cleanUrlOnDispose, }: Partial> = {}): BrowserHistory { return new BrowserHistory({ createURL, @@ -334,5 +359,6 @@ export default function historyRouter({ start, dispose, push, + cleanUrlOnDispose, }); }