diff --git a/packages/core-data/src/hooks/ProgressiveSearch.js b/packages/core-data/src/hooks/ProgressiveSearch.js index 471fd6c70..38bbdf026 100644 --- a/packages/core-data/src/hooks/ProgressiveSearch.js +++ b/packages/core-data/src/hooks/ProgressiveSearch.js @@ -1,6 +1,5 @@ // @flow -import { dequal } from 'dequal/lite'; import { useCallback, useEffect, @@ -8,6 +7,11 @@ import { useState } from 'react'; import type { SearchResult } from '../types/typesense/SearchResult'; +import { + CACHE_ACTION_RESET, + CACHE_ACTION_SKIP, + getCacheAction +} from '../utils/ProgressiveSearch'; import TypesenseUtils from '../utils/Typesense'; type OnCompleteCallback = (results: Array) => void; @@ -16,7 +20,7 @@ const useProgressiveSearch = (infiniteHits, transformResults = null) => { const [cachedHits, setCachedHits] = useState(TypesenseUtils.createCachedHits([])); const [searching, setSearching] = useState(false); - const lastSearchState = useRef(); + const lastResults = useRef(); const callbacks = useRef>([]); /** @@ -37,27 +41,6 @@ const useProgressiveSearch = (infiniteHits, transformResults = null) => { callbacks.current = callbacks.current.filter((c) => c !== callback); }, []); - /** - * Returns true if the state has changed. - * - * @param a - * @param b - * @param ignorePageNo - * - * @returns {boolean} - */ - const hasStateChanged = (a: any, b: any, ignorePageNo?: boolean) => { - if (ignorePageNo && a) { - delete a.page; - } - - if (ignorePageNo && b) { - delete b.page; - } - - return !dequal(a, b); - }; - /** * Returns the transformed hits if the callback is provided. Otherwise, the untransformed hits are returned. * @@ -75,40 +58,46 @@ const useProgressiveSearch = (infiniteHits, transformResults = null) => { return value; }; + /** + * Adds the hits from each newly received page to the cache. A first page starts the cache over; later pages + * are appended. The decision is made from the results object and its page number rather than from + * `results._state`, because InstantSearch patches that state onto the previous results while a new search is + * in flight (see `getCacheAction`). + */ useEffect(() => { - const { isFirstPage, results } = infiniteHits; - const hits = getHits(results); + const { results } = infiniteHits; + const action = getCacheAction(results, lastResults.current); - if (isFirstPage) { - setSearching(true); + if (action === CACHE_ACTION_SKIP) { + return; } - // Add to cache and load next page - if (isFirstPage && hasStateChanged(results._state, lastSearchState.current, true)) { + lastResults.current = results; + + const hits = getHits(results); + + if (action === CACHE_ACTION_RESET) { + setSearching(true); setCachedHits(() => TypesenseUtils.createCachedHits(hits)); } else { setCachedHits(({ merge }) => merge(hits)); } }, [infiniteHits.results]); + /** + * Loads the next page after each page is cached. Once the last page is in the cache, notifies the observers + * with the complete set of hits. + */ useEffect(() => { - const { isLastPage, results } = infiniteHits; - const hits = getHits(results); - - if (!isLastPage && infiniteHits.showMore) { - setTimeout(() => infiniteHits.showMore(), 25); - } else if (hasStateChanged(results._state, lastSearchState.current)) { - callbacks.current.forEach((callback) => { - const merged = cachedHits.merge(hits); - callback(merged.hits); - }); - } + const { isLastPage, showMore } = infiniteHits; - if (isLastPage) { - setSearching(false); + if (!isLastPage && showMore) { + setTimeout(() => showMore(), 25); + return; } - lastSearchState.current = results._state; + callbacks.current.forEach((callback) => callback(cachedHits.hits)); + setSearching(false); }, [cachedHits]); return { diff --git a/packages/core-data/src/utils/ProgressiveSearch.js b/packages/core-data/src/utils/ProgressiveSearch.js new file mode 100644 index 000000000..84f9b7433 --- /dev/null +++ b/packages/core-data/src/utils/ProgressiveSearch.js @@ -0,0 +1,27 @@ +// @flow + +export const CACHE_ACTION_MERGE = 'merge'; +export const CACHE_ACTION_RESET = 'reset'; +export const CACHE_ACTION_SKIP = 'skip'; + +/** + * Decides what a progressive search should do with a results object it has just been rendered with. + * + * InstantSearch re-renders widgets with the previous results while a new search is in flight, and it patches + * `results._state` on that previous object to the current state so the UI looks up to date (its "optimistic UI"). + * The state on a results object therefore does not identify the search that produced its hits. What does is the + * results object itself, which InstantSearch replaces only when a response arrives, and the page number of that + * response, which comes from the response rather than from the patched state. + * + * @param results The results the hook was rendered with. + * @param previousResults The results object the hook processed last. + * + * @returns {string} `reset` when a first page arrives, `merge` when a later page arrives, `skip` otherwise. + */ +export const getCacheAction = (results: any, previousResults: any): string => { + if (!results || results === previousResults || results.__isArtificial) { + return CACHE_ACTION_SKIP; + } + + return (results.page || 0) === 0 ? CACHE_ACTION_RESET : CACHE_ACTION_MERGE; +}; diff --git a/packages/core-data/src/utils/ProgressiveSearch.spec.js b/packages/core-data/src/utils/ProgressiveSearch.spec.js new file mode 100644 index 000000000..17cfdcf33 --- /dev/null +++ b/packages/core-data/src/utils/ProgressiveSearch.spec.js @@ -0,0 +1,41 @@ +import { getCacheAction } from './ProgressiveSearch'; + +const results = (page, extra = {}) => ({ page, hits: [], ...extra }); + +describe('getCacheAction', () => { + it('skips when there are no results yet', () => { + expect(getCacheAction(undefined, undefined)).toEqual('skip'); + expect(getCacheAction(null, undefined)).toEqual('skip'); + }); + + it('skips a re-render of the results object already processed', () => { + const first = results(0); + expect(getCacheAction(first, first)).toEqual('skip'); + }); + + it('skips the artificial results InstantSearch renders before a search completes', () => { + expect(getCacheAction(results(0, { __isArtificial: true }), undefined)).toEqual('skip'); + }); + + it('resets the cache when a first page arrives', () => { + expect(getCacheAction(results(0), undefined)).toEqual('reset'); + expect(getCacheAction(results(undefined), undefined)).toEqual('reset'); + }); + + it('merges when a later page arrives', () => { + expect(getCacheAction(results(1), results(0))).toEqual('merge'); + expect(getCacheAction(results(2), results(1))).toEqual('merge'); + }); + + it('resets again when a new search starts, even if the state was patched to look unchanged', () => { + // InstantSearch patches `_state` on the previous results to the current state + // (optimistic UI), so two different results objects can carry an identical `_state`. + // Only the page number of the received results says whether a new search began. + const state = { disjunctiveFacetsRefinements: { facet: ['value'] }, page: 0 }; + const stale = results(0, { _state: state, hits: [{ uuid: 'a' }, { uuid: 'b' }] }); + const fresh = results(0, { _state: state, hits: [{ uuid: 'a' }] }); + + expect(getCacheAction(stale, undefined)).toEqual('reset'); + expect(getCacheAction(fresh, stale)).toEqual('reset'); + }); +});