Skip to content
This repository has been archived by the owner on Dec 30, 2022. It is now read-only.

fix(infiniteHits): do not cache the cached hits #3011

Merged
merged 1 commit into from
Feb 19, 2021
Merged

fix(infiniteHits): do not cache the cached hits #3011

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
196 changes: 196 additions & 0 deletions packages/react-instantsearch-core/src/connectors/__tests__/connectInfiniteHits.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import isEqual from 'react-fast-compare';
import connect from '../connectInfiniteHits';

jest.mock('../../core/createConnector', () => x => x);
Expand Down Expand Up @@ -559,6 +560,201 @@ describe('connectInfiniteHits', () => {

expect(actual).toEqual(expectation);
});

it('read from given cache', () => {
const cache = {
read: jest.fn(),
write: jest.fn(),
};
const props = { cache, contextValue };
const searchState = {};
const searchResults = {};
connect.getProvidedProps.call({}, props, searchState, searchResults);
expect(cache.read).toHaveBeenCalledTimes(1);
});

it('read from new cache prop', () => {
const cache = {
read: jest.fn(),
write: jest.fn(),
};
const searchState = {};
const searchResults = {};
const instance = {};
connect.getProvidedProps.call(
instance,
{ cache, contextValue },
searchState,
searchResults
);
expect(cache.read).toHaveBeenCalledTimes(1);

const cache2 = {
read: jest.fn(),
write: jest.fn(),
};
connect.getProvidedProps.call(
instance,
{ cache: cache2, contextValue },
searchState,
searchResults
);
expect(cache.read).toHaveBeenCalledTimes(1);
expect(cache2.read).toHaveBeenCalledTimes(1);
});

it('keep the same in-memory cache', () => {
const searchState = {};
const searchResults = {};
const instance = {};
connect.getProvidedProps.call(
instance,
{ contextValue },
searchState,
searchResults
);
const memoryCache = instance._cache;

connect.getProvidedProps.call(
instance,
{ contextValue },
searchState,
searchResults
);
expect(instance._cache).toBe(memoryCache);
});

it('render hits correctly after invalidating cache', () => {
const getStateWithoutPage = state => {
const { page, ...rest } = state || {};
return rest;
};

const getInMemoryCache = () => {
let cachedHits = undefined;
let cachedState = undefined;
return {
read({ state }) {
return isEqual(cachedState, getStateWithoutPage(state))
? cachedHits
: null;
},
write({ state, hits }) {
cachedState = getStateWithoutPage(state);
cachedHits = hits;
},
clear() {
cachedHits = undefined;
cachedState = undefined;
},
};
};

const instance = {};
const cache = getInMemoryCache();
const props = { cache, contextValue };
const searchState = {};
const searchResults1 = {
results: {
hits: [{ objectID: 'a' }, { objectID: 'b' }, { objectID: 'c' }],
hitsPerPage: 3,
page: 0,
nbPages: 3,
},
};
expect(
connect.getProvidedProps.call(
instance,
props,
searchState,
searchResults1
).hits
).toMatchInlineSnapshot(`
Array [
Object {
"__position": 1,
"objectID": "a",
},
Object {
"__position": 2,
"objectID": "b",
},
Object {
"__position": 3,
"objectID": "c",
},
]
`);

const searchResults2 = {
results: {
hits: [{ objectID: 'd' }, { objectID: 'e' }, { objectID: 'f' }],
hitsPerPage: 3,
page: 1,
nbPages: 3,
},
};
expect(
connect.getProvidedProps.call(
instance,
props,
searchState,
searchResults2
).hits
).toMatchInlineSnapshot(`
Array [
Object {
"__position": 1,
"objectID": "a",
},
Object {
"__position": 2,
"objectID": "b",
},
Object {
"__position": 3,
"objectID": "c",
},
Object {
"__position": 4,
"objectID": "d",
},
Object {
"__position": 5,
"objectID": "e",
},
Object {
"__position": 6,
"objectID": "f",
},
]
`);

cache.clear();
expect(
connect.getProvidedProps.call(
instance,
props,
searchState,
searchResults2
).hits
).toMatchInlineSnapshot(`
Array [
Object {
"__position": 4,
"objectID": "d",
},
Object {
"__position": 5,
"objectID": "e",
},
Object {
"__position": 6,
"objectID": "f",
},
]
`);
});
});

describe('multi index', () => {
Expand Down
32 changes: 14 additions & 18 deletions packages/react-instantsearch-core/src/connectors/connectInfiniteHits.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,12 @@ export default createConnector({

this._prevState = this._prevState || {};

const cache = props.cache || getInMemoryCache();
if (this._cachedHits === undefined) {
this._cachedHits = cache.read({ state: searchState }) || {};
}
this._cache = props.cache ? props.cache : this._cache || getInMemoryCache();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a very important fix!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it especially makes sure to take cache prop into account even when user passes a different object!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and makes sure we don't recreate the memory cache on every render

let cachedHits = this._cache.read({ state: searchState }) || {};

if (!results) {
return {
hits: extractHitsFromCachedHits(this._cachedHits),
hits: extractHitsFromCachedHits(cachedHits),
hasPrevious: false,
hasMore: false,
refine: () => {},
Expand All @@ -111,24 +109,20 @@ export default createConnector({
);

if (!isEqual(currentState, this._prevState)) {
this._cachedHits = cache.read({ state: searchState }) || {};
cachedHits = this._cache.read({ state: searchState }) || {};
}
if (this._cachedHits[page] === undefined) {
this._cachedHits[page] = hitsWithPositionsAndQueryID;
cache.write({ state: searchState, hits: this._cachedHits });
if (cachedHits[page] === undefined) {
cachedHits[page] = hitsWithPositionsAndQueryID;
this._cache.write({ state: searchState, hits: cachedHits });
}

this._prevState = currentState;
/*
Math.min() and Math.max() returns Infinity or -Infinity when no argument is given.
But there is always something in this point because of `this._cachedHits[page]`.
But there is always something in this point because of `cachedHits[page]`.
*/
const firstReceivedPage = Math.min(
...Object.keys(this._cachedHits).map(Number)
);
const lastReceivedPage = Math.max(
...Object.keys(this._cachedHits).map(Number)
);
const firstReceivedPage = Math.min(...Object.keys(cachedHits).map(Number));
const lastReceivedPage = Math.max(...Object.keys(cachedHits).map(Number));

const hasPrevious = firstReceivedPage > 0;
const lastPageIndex = nbPages - 1;
Expand All @@ -137,7 +131,7 @@ export default createConnector({
const refineNext = event => this.refine(event, lastReceivedPage + 1);

return {
hits: extractHitsFromCachedHits(this._cachedHits),
hits: extractHitsFromCachedHits(cachedHits),
hasPrevious,
hasMore,
refinePrevious,
Expand All @@ -156,7 +150,9 @@ export default createConnector({
},

refine(props, searchState, event, index) {
const pages = Object.keys(this._cachedHits || {}).map(Number);
this._cache = props.cache ? props.cache : this._cache || getInMemoryCache();
const cachedHits = this._cache.read({ state: searchState }) || {};
const pages = Object.keys(cachedHits).map(Number);
const lastReceivedPage =
pages.length === 0 ? undefined : Math.max(...pages);
// If there is no key in `this._cachedHits`,
Expand Down