From 625ca9bd15df3cbb9aa3ce0353039ebdda1854f8 Mon Sep 17 00:00:00 2001 From: Haroen Viaene Date: Tue, 31 Dec 2024 11:37:32 +0100 Subject: [PATCH] fix(instantsearch): remove searchFunction This option is deprecated and all use cases are possible with onStateChange or other methods too [FX-3216] BREAKING CHANGE: replace searchFunction usage with onStateChange --- .../src/__tests__/RoutingManager.test.ts | 111 ------------------ .../src/__tests__/instantsearch.test.tsx | 46 -------- .../instantsearch-core/src/instantsearch.ts | 39 ------ .../src/types/instantsearch.ts | 8 -- .../stories/instantsearch.stories.ts | 8 +- .../__tests__/InstantSearch.test.tsx | 49 -------- .../src/lib/useInstantSearchApi.ts | 8 -- .../src/components/InstantSearch.js | 5 - .../src/components/__tests__/InstantSearch.js | 25 ---- .../src/util/createInstantSearchComponent.js | 4 - .../stories/InstantSearch.stories.js | 14 --- 11 files changed, 4 insertions(+), 313 deletions(-) diff --git a/packages/instantsearch-core/src/__tests__/RoutingManager.test.ts b/packages/instantsearch-core/src/__tests__/RoutingManager.test.ts index df91c4c5bb..d50443968e 100644 --- a/packages/instantsearch-core/src/__tests__/RoutingManager.test.ts +++ b/packages/instantsearch-core/src/__tests__/RoutingManager.test.ts @@ -186,68 +186,6 @@ describe('RoutingManager', () => { }); }); - test('should apply state mapping on differences after searchFunction', async () => { - const searchClient = createSearchClient(); - - const router = createFakeRouter({ - write: jest.fn(), - }); - - const stateMapping = createFakeStateMapping({ - stateToRoute(uiState) { - return Object.keys(uiState).reduce((state, indexId) => { - const indexState = uiState[indexId]; - - return { - ...state, - [indexId]: { - query: indexState.query && indexState.query.toUpperCase(), - }, - }; - }, {}); - }, - }); - - const search = instantsearch({ - indexName: 'indexName', - searchFunction: (helper) => { - helper.setQuery('test').search(); - }, - searchClient, - routing: { - stateMapping, - router, - }, - }); - - search.addWidgets([ - createWidget({ - getWidgetUiState(uiState, { searchParameters }) { - return { - ...uiState, - query: searchParameters.query, - }; - }, - getWidgetSearchParameters: jest.fn( - (searchParameters) => searchParameters - ), - }), - ]); - - search.start(); - - await wait(0); - // initialization is done at this point - - expect(search.mainIndex.getHelper()!.state.query).toEqual('test'); - - expect(router.write).toHaveBeenLastCalledWith({ - indexName: { - query: 'TEST', - }, - }); - }); - test('should keep the UI state up to date on state changes', async () => { const searchClient = createSearchClient(); const stateMapping = createFakeStateMapping({}); @@ -299,55 +237,6 @@ describe('RoutingManager', () => { expect(router.write).toHaveBeenCalledTimes(1); }); - test('should keep the UI state up to date on first render', async () => { - const searchClient = createSearchClient(); - const stateMapping = createFakeStateMapping({}); - const router = createFakeRouter({ - write: jest.fn(), - }); - - const search = instantsearch({ - indexName: 'indexName', - searchFunction(helper) { - // Force the value of the query - helper.setQuery('Apple iPhone').search(); - }, - searchClient, - routing: { - router, - stateMapping, - }, - }); - - const fakeSearchBox = connectSearchBox(() => {})({}); - const fakeHitsPerPage = connectHitsPerPage(() => {})({ - items: [{ default: true, value: 1, label: 'one' }], - }); - - search.addWidgets([fakeSearchBox, fakeHitsPerPage]); - - // Trigger the call to `searchFunction` -> Apple iPhone - search.start(); - - await wait(0); - - expect(router.write).toHaveBeenCalledTimes(1); - expect(router.write).toHaveBeenLastCalledWith({ - indexName: { - query: 'Apple iPhone', - }, - }); - - // Trigger change - search.removeWidgets([fakeHitsPerPage]); - - await wait(0); - - // 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 () => { const searchClient = createSearchClient(); const stateMapping = createFakeStateMapping({}); diff --git a/packages/instantsearch-core/src/__tests__/instantsearch.test.tsx b/packages/instantsearch-core/src/__tests__/instantsearch.test.tsx index b5d6477f23..04ca20e9fa 100644 --- a/packages/instantsearch-core/src/__tests__/instantsearch.test.tsx +++ b/packages/instantsearch-core/src/__tests__/instantsearch.test.tsx @@ -824,52 +824,6 @@ describe('start', () => { ).toHaveBeenCalledTimes(1); }); - it('calls the provided `searchFunction` with a single request', async () => { - const searchFunction = jest.fn((helper) => - helper.setQuery('test').search() - ); - const searchClient = createSearchClient(); - const search = new InstantSearch({ - indexName: 'indexName', - searchFunction, - searchClient, - }); - - search.addWidgets([virtualSearchBox({})]); - - expect(searchFunction).toHaveBeenCalledTimes(0); - expect(searchClient.search).toHaveBeenCalledTimes(0); - - search.start(); - - await wait(0); - - expect(searchFunction).toHaveBeenCalledTimes(1); - expect(searchClient.search).toHaveBeenCalledTimes(1); - expect(search.mainIndex.getHelper()!.state.query).toBe('test'); - }); - - it('calls the provided `searchFunction` with multiple requests', () => { - const searchClient = createSearchClient(); - const search = new InstantSearch({ - indexName: 'indexName', - searchClient, - searchFunction(helper) { - const nextState = helper.state - .addDisjunctiveFacet('brand') - .addDisjunctiveFacetRefinement('brand', 'Apple'); - - helper.setState(nextState).search(); - }, - }); - - search.addWidgets([virtualSearchBox({})]); - - expect(() => { - search.start(); - }).not.toThrow(); - }); - it('forwards the `initialUiState` to the main index', () => { const search = new InstantSearch({ indexName: 'indexName', diff --git a/packages/instantsearch-core/src/instantsearch.ts b/packages/instantsearch-core/src/instantsearch.ts index 22a727747a..b9a93dee37 100644 --- a/packages/instantsearch-core/src/instantsearch.ts +++ b/packages/instantsearch-core/src/instantsearch.ts @@ -24,7 +24,6 @@ import version from './version'; import { index } from './widgets/index-widget'; import type { - SearchClient, Widget, IndexWidget, UiState, @@ -75,7 +74,6 @@ export class InstantSearch< _initialUiState: TUiState; _initialResults: InitialResults | null; _createURL: CreateURL; - _searchFunction?: InstantSearchOptions['searchFunction']; _mainHelperSearch?: AlgoliaSearchHelper['search']; _hasSearchWidget: boolean = false; _hasRecommendWidget: boolean = false; @@ -106,7 +104,6 @@ export class InstantSearch< initialUiState = {} as TUiState, routing = null, insights = undefined, - searchFunction, stalledSearchDelay = 200, searchClient = null, onStateChange = null, @@ -170,14 +167,6 @@ See ${createDocumentationLink({ this._insights = insights; - if (searchFunction) { - warning( - false, - `The \`searchFunction\` option is deprecated. Use \`onStateChange\` instead.` - ); - this._searchFunction = searchFunction; - } - this.sendEventToInsights = noop; if (routing) { @@ -336,34 +325,6 @@ See ${createDocumentationLink({ return mainHelper; }; - if (this._searchFunction) { - // this client isn't used to actually search, but required for the helper - // to not throw errors - const fakeClient = { - search: () => new Promise(noop), - } as any as SearchClient; - - this._mainHelperSearch = mainHelper.search.bind(mainHelper); - mainHelper.search = () => { - const mainIndexHelper = this.mainIndex.getHelper()!; - const searchFunctionHelper = algoliasearchHelper( - fakeClient, - mainIndexHelper.state.index, - mainIndexHelper.state - ); - searchFunctionHelper.once('search', ({ state }) => { - mainIndexHelper.overrideStateWithoutTriggeringChangeEvent(state); - this._mainHelperSearch!(); - }); - // Forward state changes from `searchFunctionHelper` to `mainIndexHelper` - searchFunctionHelper.on('change', ({ state }) => { - mainIndexHelper.setState(state); - }); - this._searchFunction!(searchFunctionHelper); - return mainHelper; - }; - } - // Only the "main" Helper emits the `error` event vs the one for `search` // and `results` that are also emitted on the derived one. mainHelper.on('error', (error) => { diff --git a/packages/instantsearch-core/src/types/instantsearch.ts b/packages/instantsearch-core/src/types/instantsearch.ts index 2454c28e2e..af600de60d 100644 --- a/packages/instantsearch-core/src/types/instantsearch.ts +++ b/packages/instantsearch-core/src/types/instantsearch.ts @@ -2,7 +2,6 @@ import type { SearchClient } from './algoliasearch'; import type { InsightsProps } from './insights'; import type { RouterProps } from './router'; import type { UiState } from './ui-state'; -import type { AlgoliaSearchHelper } from 'algoliasearch-helper'; export type InstantSearchOptions< TUiState extends UiState = UiState, @@ -45,13 +44,6 @@ export type InstantSearchOptions< * to `Number.prototype.toLocaleString()` */ numberLocale?: string; - /** - * A hook that will be called each time a search needs to be done, with the - * helper as a parameter. It's your responsibility to call `helper.search()`. - * This option allows you to avoid doing searches at page load for example. - * @deprecated use onStateChange instead - */ - searchFunction?: (helper: AlgoliaSearchHelper) => void; /** * Function called when the state changes. * diff --git a/packages/instantsearch.js/stories/instantsearch.stories.ts b/packages/instantsearch.js/stories/instantsearch.stories.ts index 3ae6959993..04272715a8 100644 --- a/packages/instantsearch.js/stories/instantsearch.stories.ts +++ b/packages/instantsearch.js/stories/instantsearch.stories.ts @@ -4,16 +4,16 @@ import { withHits } from '../.storybook/decorators'; storiesOf('Basics/InstantSearch', module) .add( - 'with searchFunction to prevent search', + 'with onStateChange to prevent search', withHits(() => {}, { - searchFunction: (helper) => { - const query = helper.state.query; + onStateChange({ uiState, setUiState }) { + const query = uiState.instant_search?.query ?? ''; if (query === '') { return; } - helper.search(); + setUiState(uiState); }, }) ) diff --git a/packages/react-instantsearch-core/src/components/__tests__/InstantSearch.test.tsx b/packages/react-instantsearch-core/src/components/__tests__/InstantSearch.test.tsx index 7293be78b8..60359b5b3c 100644 --- a/packages/react-instantsearch-core/src/components/__tests__/InstantSearch.test.tsx +++ b/packages/react-instantsearch-core/src/components/__tests__/InstantSearch.test.tsx @@ -775,55 +775,6 @@ describe('InstantSearch', () => { }); }); - test('updates searchFunction on searchFunction prop change', async () => { - const searchClient = createAlgoliaSearchClient({}); - const searchFunction1 = jest.fn((helper) => { - helper.search(); - }); - const searchFunction2 = jest.fn((helper) => { - helper.search(); - }); - - function App({ - searchFunction, - }: Pick) { - return ( - - - - - - ); - } - - const { rerender } = render(); - - await waitFor(() => { - expect(searchFunction1).toHaveBeenCalledTimes(1); - }); - - userEvent.type(screen.getByRole('searchbox'), 'iphone'); - - await waitFor(() => { - expect(searchFunction1).toHaveBeenCalledTimes(7); - }); - - rerender(); - - userEvent.type(screen.getByRole('searchbox'), ' case', { - initialSelectionStart: 6, - }); - - await waitFor(() => { - expect(searchFunction1).toHaveBeenCalledTimes(7); - expect(searchFunction2).toHaveBeenCalledTimes(5); - }); - }); - test('triggers no search on unmount', async () => { const searchClient = createAlgoliaSearchClient({}); diff --git a/packages/react-instantsearch-core/src/lib/useInstantSearchApi.ts b/packages/react-instantsearch-core/src/lib/useInstantSearchApi.ts index a4954c1dab..45c45d47d6 100644 --- a/packages/react-instantsearch-core/src/lib/useInstantSearchApi.ts +++ b/packages/react-instantsearch-core/src/lib/useInstantSearchApi.ts @@ -163,14 +163,6 @@ export function useInstantSearchApi( prevPropsRef.current = props; } - if (prevProps.searchFunction !== props.searchFunction) { - // Updating the `searchFunction` to `undefined` is not supported by - // InstantSearch.js, so it will throw an error. - // This is a fair behavior until we add an update API in InstantSearch.js. - search._searchFunction = props.searchFunction; - prevPropsRef.current = props; - } - if (prevProps.stalledSearchDelay !== props.stalledSearchDelay) { // The default `stalledSearchDelay` in InstantSearch.js is 200ms. // We need to reset it when it's undefined to get back to the original value. diff --git a/packages/vue-instantsearch/src/components/InstantSearch.js b/packages/vue-instantsearch/src/components/InstantSearch.js index 43eb3c55c5..8a0d1af85a 100644 --- a/packages/vue-instantsearch/src/components/InstantSearch.js +++ b/packages/vue-instantsearch/src/components/InstantSearch.js @@ -49,10 +49,6 @@ export default createInstantSearchComponent({ type: Number, default: undefined, }, - searchFunction: { - type: Function, - default: undefined, - }, onStateChange: { type: Function, default: undefined, @@ -98,7 +94,6 @@ export default createInstantSearchComponent({ indexName: this.indexName, routing: this.routing, stalledSearchDelay: this.stalledSearchDelay, - searchFunction: this.searchFunction, onStateChange: this.onStateChange, initialUiState: this.initialUiState, future: this.future, diff --git a/packages/vue-instantsearch/src/components/__tests__/InstantSearch.js b/packages/vue-instantsearch/src/components/__tests__/InstantSearch.js index df55ed7f57..552b6ae4a3 100644 --- a/packages/vue-instantsearch/src/components/__tests__/InstantSearch.js +++ b/packages/vue-instantsearch/src/components/__tests__/InstantSearch.js @@ -42,7 +42,6 @@ beforeEach(() => { it('passes props to InstantSearch.js', () => { const searchClient = createSearchClient(); - const searchFunction = (helper) => helper.search(); const routing = { router: historyRouter(), stateMapping: simpleStateMapping(), @@ -54,7 +53,6 @@ it('passes props to InstantSearch.js', () => { indexName: 'something', routing, stalledSearchDelay: 250, - searchFunction, }, }); @@ -62,7 +60,6 @@ it('passes props to InstantSearch.js', () => { indexName: 'something', routing, searchClient, - searchFunction, stalledSearchDelay: 250, }); }); @@ -247,33 +244,11 @@ it('does not warn when the `search-client` does not change', async () => { expect(warn).not.toHaveBeenCalled(); }); -it('Allows a change in `search-function`', async () => { - const oldValue = () => {}; - const newValue = () => {}; - - const wrapper = mount(InstantSearch, { - propsData: { - searchClient: createSearchClient(), - indexName: 'bla', - searchFunction: oldValue, - }, - }); - - expect(wrapper.vm.instantSearchInstance._searchFunction).toEqual(oldValue); - - await wrapper.setProps({ - searchFunction: newValue, - }); - - expect(wrapper.vm.instantSearchInstance._searchFunction).toEqual(newValue); -}); - it('Allows a change in `stalled-search-delay`', async () => { const wrapper = mount(InstantSearch, { propsData: { searchClient: createSearchClient(), indexName: 'bla', - searchFunction: () => {}, stalledSearchDelay: 200, }, }); diff --git a/packages/vue-instantsearch/src/util/createInstantSearchComponent.js b/packages/vue-instantsearch/src/util/createInstantSearchComponent.js index cdd02851ae..b19e41dc71 100644 --- a/packages/vue-instantsearch/src/util/createInstantSearchComponent.js +++ b/packages/vue-instantsearch/src/util/createInstantSearchComponent.js @@ -54,10 +54,6 @@ export const createInstantSearchComponent = (component) => ({ 'Please open a new issue: https://github.com/algolia/instantsearch/discussions/new?category=ideas&labels=triage%2cLibrary%3A+Vue+InstantSearch&title=Feature%20request%3A%20dynamic%20props' ); }, - searchFunction(searchFunction) { - // private InstantSearch.js API: - this.instantSearchInstance._searchFunction = searchFunction; - }, middlewares: { immediate: true, handler(next, prev) { diff --git a/packages/vue-instantsearch/stories/InstantSearch.stories.js b/packages/vue-instantsearch/stories/InstantSearch.stories.js index 489d20f3c2..7cc978b1ee 100644 --- a/packages/vue-instantsearch/stories/InstantSearch.stories.js +++ b/packages/vue-instantsearch/stories/InstantSearch.stories.js @@ -29,13 +29,6 @@ const clients = { fake: new FakeClient(), }; -const searchFunctions = { - working: undefined, - broken(helper) { - helper.setQuery('bingo').search(); - }, -}; - storiesOf('ais-instant-search', module) .add('simple usage', () => ({ template: ` @@ -74,7 +67,6 @@ storiesOf('ais-instant-search', module)

This is inside a ais-instant-search: {{indexName}}

@@ -91,8 +83,6 @@ storiesOf('ais-instant-search', module) return { searchClientName: 'working', searchClient: clients.working, - searchFunctionName: 'working', - searchFunction: searchFunctions.working, stalledSearchDelay: undefined, indexName: 'instant_search', }; @@ -102,9 +92,5 @@ storiesOf('ais-instant-search', module) this.searchClientName = newName; this.searchClient = clients[newName]; }, - searchFunctionName(newName) { - this.searchFunctionName = newName; - this.searchFunction = searchFunctions[newName]; - }, }, }));