From 5ff4c8307c2be7bde7fb53aa9935a243e6532fe2 Mon Sep 17 00:00:00 2001 From: Haroen Viaene Date: Wed, 13 May 2020 15:43:01 +0200 Subject: [PATCH] feat(index): support adding index widget with initial UI state (#4359) 1. when hydrating (server or client) we add widgets, these shouldn't search yet, until started since every added widget calls scheduleSearch if init has happened, this used to do a search, even before start was called, and thus with not yet the correct state 2. make sure local ui state keeps "not yet mounted" widgets on change e.g. initialUiState contains a few widgets, if you mount those one by one, after init on its index, each widget would cause a change event on the helper, which used to erase non-valid state, since it starts from an empty ui state. This is now fixed by using the previous local ui state as base, but a special provision for isPageReset 3. prevent mutation in escapeHits It's okay to do results.hits = ... inside connectHits, since that overrides just the value on SearchResults, however if we mutate within the inner hits, we are actually mutating _rawResults, which isn't what we want to achieve for server side rendering, since _rawResults gets serialised. Further changes are just minor for tests --- src/lib/InstantSearch.ts | 4 +- src/lib/__tests__/escape-highlight-test.js | 22 +++++ src/lib/escape-highlight.ts | 12 ++- src/widgets/index/__tests__/index-test.ts | 109 +++++++++++++++++++++ src/widgets/index/index.ts | 76 +++++++++++--- test/mock/createInstantSearch.ts | 12 +-- 6 files changed, 208 insertions(+), 27 deletions(-) diff --git a/src/lib/InstantSearch.ts b/src/lib/InstantSearch.ts index 86a706f35d..0ec703b670 100644 --- a/src/lib/InstantSearch.ts +++ b/src/lib/InstantSearch.ts @@ -483,7 +483,9 @@ See ${createDocumentationLink({ } public scheduleSearch = defer(() => { - this.mainHelper!.search(); + if (this.started) { + this.mainHelper!.search(); + } }); public scheduleRender = defer(() => { diff --git a/src/lib/__tests__/escape-highlight-test.js b/src/lib/__tests__/escape-highlight-test.js index fa52ace5c2..5354667144 100644 --- a/src/lib/__tests__/escape-highlight-test.js +++ b/src/lib/__tests__/escape-highlight-test.js @@ -246,4 +246,26 @@ describe('escapeHits()', () => { expect(hits).toEqual(output); }); + + it('should not mutate the hit', () => { + const hit = { + _highlightResult: { + foobar: { + value: '', + }, + }, + }; + + const hits = [hit]; + + escapeHits(hits); + + expect(hit).toEqual({ + _highlightResult: { + foobar: { + value: '', + }, + }, + }); + }); }); diff --git a/src/lib/escape-highlight.ts b/src/lib/escape-highlight.ts index a8550d7585..680e7af238 100644 --- a/src/lib/escape-highlight.ts +++ b/src/lib/escape-highlight.ts @@ -46,13 +46,15 @@ function recursiveEscape(input: any): any { export default function escapeHits(hits: Hit[]): Hit[] { if ((hits as any).__escaped === undefined) { - hits = hits.map(hit => { - if (hit._highlightResult) { - hit._highlightResult = recursiveEscape(hit._highlightResult); + // We don't override the value on hit because it will mutate the raw results + // instead we make a shallow copy and we assign the escaped values on it. + hits = hits.map(({ _highlightResult, _snippetResult, ...hit }) => { + if (_highlightResult) { + hit._highlightResult = recursiveEscape(_highlightResult); } - if (hit._snippetResult) { - hit._snippetResult = recursiveEscape(hit._snippetResult); + if (_snippetResult) { + hit._snippetResult = recursiveEscape(_snippetResult); } return hit; diff --git a/src/widgets/index/__tests__/index-test.ts b/src/widgets/index/__tests__/index-test.ts index a4792348fc..0a2804dcd1 100644 --- a/src/widgets/index/__tests__/index-test.ts +++ b/src/widgets/index/__tests__/index-test.ts @@ -75,6 +75,7 @@ describe('index', () => { }), getWidgetState(uiState) { return { + ...uiState, configure: { ...uiState.configure, ...params, @@ -255,6 +256,66 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/index-widge }); }); + it('forwards initial UiState to inner indices', () => { + const instance = index({ indexName: 'indexName' }); + const instantSearchInstance = createInstantSearch({ + _initialUiState: { + indexName: { + query: 'xxx', + }, + two: { + query: 'inner', + }, + }, + }); + const inner = index({ indexName: 'two' }); + jest.spyOn(inner, 'init'); + + const widgets = [createSearchBox(), createPagination(), inner]; + const innerWidgets = [createSearchBox()]; + + instance.init( + createInitOptions({ + instantSearchInstance, + parent: null, + }) + ); + + widgets.forEach(widget => { + expect(widget.init).toHaveBeenCalledTimes(0); + }); + + instance.addWidgets(widgets); + + widgets.forEach(widget => { + expect(widget.init).toHaveBeenCalledTimes(1); + expect(widget.init).toHaveBeenCalledWith({ + instantSearchInstance, + parent: instance, + uiState: { + indexName: { + query: 'xxx', + }, + two: { + query: 'inner', + }, + }, + helper: instance.getHelper(), + state: instance.getHelper()!.state, + templatesConfig: instantSearchInstance.templatesConfig, + createURL: expect.any(Function), + }); + }); + + inner.addWidgets(innerWidgets); + + expect(inner.getWidgetState({})).toEqual({ + two: { + query: 'inner', + }, + }); + }); + it('schedules a search to take the added widgets into account', () => { const instance = index({ indexName: 'indexName' }); const instantSearchInstance = createInstantSearch({ @@ -1783,6 +1844,54 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/index-widge }); }); }); + + it('uiState on inner index does not get erased on addWidget', () => { + const level0 = index({ indexName: 'level0IndexName' }); + + const searchClient = createSearchClient(); + const mainHelper = algoliasearchHelper(searchClient, '', {}); + const instantSearchInstance = createInstantSearch({ + mainHelper, + _initialUiState: { + level0IndexName: { + query: 'something', + }, + }, + }); + + instantSearchInstance.mainIndex.init( + createInitOptions({ instantSearchInstance, parent: null }) + ); + + instantSearchInstance.mainIndex.addWidgets([level0]); + + level0.init( + createInitOptions({ + instantSearchInstance, + parent: instantSearchInstance.mainIndex, + }) + ); + + level0.addWidgets([ + createConfigure({ + distinct: false, + }), + ]); + + level0.addWidgets([createSearchBox()]); + + expect(level0.getHelper()!.state.query).toBe('something'); + + expect(instantSearchInstance.mainIndex.getWidgetState({})).toEqual({ + indexName: {}, + level0IndexName: { + configure: { + distinct: false, + }, + query: 'something', + }, + }); + }); }); describe('render', () => { diff --git a/src/widgets/index/index.ts b/src/widgets/index/index.ts index 689744d140..9bce5b394b 100644 --- a/src/widgets/index/index.ts +++ b/src/widgets/index/index.ts @@ -4,6 +4,7 @@ import algoliasearchHelper, { PlainSearchParameters, SearchParameters, SearchResults, + AlgoliaSearchHelper, } from 'algoliasearch-helper'; import { InstantSearch, @@ -68,9 +69,39 @@ export function isIndexWidget(widget: Widget): widget is Index { return widget.$$type === 'ais.index'; } +/** + * This is the same content as helper._change / setState, but allowing for extra + * UiState to be synchronized. + * see: https://github.com/algolia/algoliasearch-helper-js/blob/6b835ffd07742f2d6b314022cce6848f5cfecd4a/src/algoliasearch.helper.js#L1311-L1324 + */ +function privateHelperSetState( + helper: AlgoliaSearchHelper, + { + state, + isPageReset, + _uiState, + }: { + state: SearchParameters; + isPageReset?: boolean; + _uiState?: IndexUiState; + } +) { + if (state !== helper.state) { + helper.state = state; + + helper.emit('change', { + state: helper.state, + results: helper.lastResults, + isPageReset, + _uiState, + }); + } +} + function getLocalWidgetsState( widgets: Widget[], - widgetStateOptions: WidgetStateOptions + widgetStateOptions: WidgetStateOptions, + initialUiState: IndexUiState = {} ): IndexUiState { return widgets .filter(widget => !isIndexWidget(widget)) @@ -80,7 +111,7 @@ function getLocalWidgetsState( } return widget.getWidgetState(uiState, widgetStateOptions); - }, {}); + }, initialUiState); } function getLocalWidgetsSearchParameters( @@ -110,8 +141,11 @@ function resetPageFromWidgets(widgets: Widget[]): void { indexWidgets.forEach(widget => { const widgetHelper = widget.getHelper()!; - // @ts-ignore @TODO: remove "ts-ignore" once `resetPage()` is typed in the helper - widgetHelper.setState(widgetHelper.state.resetPage()); + privateHelperSetState(widgetHelper, { + // @ts-ignore @TODO: remove "ts-ignore" once `resetPage()` is typed in the helper + state: widgetHelper.state.resetPage(), + isPageReset: true, + }); resetPageFromWidgets(widget.getWidgets()); }); @@ -213,19 +247,20 @@ const index = (props: IndexProps): Index => { localWidgets = localWidgets.concat(widgets); if (localInstantSearchInstance && Boolean(widgets.length)) { - helper!.setState( - getLocalWidgetsSearchParameters(localWidgets, { + privateHelperSetState(helper!, { + state: getLocalWidgetsSearchParameters(localWidgets, { uiState: localUiState, initialSearchParameters: helper!.state, - }) - ); + }), + _uiState: localUiState, + }); widgets.forEach(widget => { if (localInstantSearchInstance && widget.init) { widget.init({ helper: helper!, parent: this, - uiState: {}, + uiState: localInstantSearchInstance._initialUiState, instantSearchInstance: localInstantSearchInstance, state: helper!.state, templatesConfig: localInstantSearchInstance.templatesConfig, @@ -402,11 +437,24 @@ const index = (props: IndexProps): Index => { // widgets. When the subscription happens before the `init` step, the (static) // configuration of the widget is pushed in the URL. That's what we want to avoid. // https://github.com/algolia/instantsearch.js/pull/994/commits/4a672ae3fd78809e213de0368549ef12e9dc9454 - helper.on('change', ({ state }) => { - localUiState = getLocalWidgetsState(localWidgets, { - searchParameters: state, - helper: helper!, - }); + helper.on('change', event => { + const { state, isPageReset } = event; + + // @ts-ignore _uiState comes from privateHelperSetState and thus isn't typed on the helper event + const _uiState = event._uiState; + + if (isPageReset) { + localUiState.page = undefined; + } + + localUiState = getLocalWidgetsState( + localWidgets, + { + searchParameters: state, + helper: helper!, + }, + _uiState || {} + ); // We don't trigger an internal change when controlled because it // becomes the responsibility of `setUiState`. diff --git a/test/mock/createInstantSearch.ts b/test/mock/createInstantSearch.ts index f67c522aae..5454859e0c 100644 --- a/test/mock/createInstantSearch.ts +++ b/test/mock/createInstantSearch.ts @@ -12,19 +12,17 @@ export const createInstantSearch = ( const mainHelper = algoliasearchHelper(searchClient, indexName, {}); const mainIndex = index({ indexName }); - let started = false; - return { indexName, mainIndex, mainHelper, client: searchClient, - started, - start: () => { - started = true; + started: false, + start() { + this.started = true; }, - dispose: () => { - started = false; + dispose() { + this.started = false; }, refresh: jest.fn(), helper: mainHelper, // @TODO: use the Helper from the index once the RoutingManger uses the index