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

Commit c071776

Browse files
HaroenvHaroenv
authored
feat(dynamicWidgets): send facets * and maxValuesPerFacet by default (#3242)
* feat(dynamicWidgets): send facets * and maxValuesPerFacet by default FX-622 <!-- Thanks for submitting a pull request! Please provide enough information so that others can review your pull request. --> **Summary** <!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? Are there any linked issues? --> The default additional search parameters of `{ facets: ['*'], maxValuesPerFacet: 20 }` prevents an extra network request when widgets mount/unmount, as both queries will be identical, and thus cached. If a refinement is set, this still results in two network requests, as the refinements can't be applied before it's known if the widget should be mounted. **Result** <!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes UI. You will be able to test out these changes on the deploy preview (address will be commented by a bot): 1. the documentation site (/) 2. a widget playground (/stories) --> When dynamic widgets is used without an explicit configure widget with `facets: ['*']` and `maxValuesPerFacet: highestLimit` an extra network request happens on load, which has a negative impact on performance. With the new additional parameters default value, widgets mounting or unmounting no longer require a separate network request Two new parameters are added to `connectDynamicWidgets` and `dynamicWidgets`: ```js <DynamicWidgets // ... other parameters // overridden with the default value facets={['*']} maxValuesPerFacet={20} /> ``` The default behaviour can also be avoided, and the previous default (non-matching parameters, which does two network requests) can be done like this: ```js <DynamicWidgets // ... other parameters // overridden default to ensure two requests are sent, // and that not all facets are requested facets={[]} /> ``` * forbid parameters other than [] and [*] * update size due to warnings
1 parent a1fa596 commit c071776

File tree

4 files changed

+310
-41
lines changed

4 files changed

+310
-41
lines changed

‎package.json‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -146,23 +146,23 @@
146146
},
147147
{
148148
"path": "packages/react-instantsearch/dist/umd/Connectors.min.js",
149-
"maxSize": "25.25 kB"
149+
"maxSize": "25.75 kB"
150150
},
151151
{
152152
"path": "packages/react-instantsearch/dist/umd/Dom.min.js",
153-
"maxSize": "42 kB"
153+
"maxSize": "42.25 kB"
154154
},
155155
{
156156
"path": "packages/react-instantsearch-core/dist/umd/ReactInstantSearchCore.min.js",
157-
"maxSize": "28.75 kB"
157+
"maxSize": "29 kB"
158158
},
159159
{
160160
"path": "packages/react-instantsearch-hooks/dist/umd/ReactInstantSearchHooks.min.js",
161161
"maxSize": "37 kB"
162162
},
163163
{
164164
"path": "packages/react-instantsearch-dom/dist/umd/ReactInstantSearchDOM.min.js",
165-
"maxSize": "44.50 kB"
165+
"maxSize": "44.75 kB"
166166
},
167167
{
168168
"path": "packages/react-instantsearch-dom-maps/dist/umd/ReactInstantSearchDOMMaps.min.js",

‎packages/react-instantsearch-core/src/connectors/__tests__/connectDynamicWidgets.js‎

Lines changed: 206 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,81 @@ const EMPTY_RESPONSE = {
2424

2525
describe('connectDynamicWidgets', () => {
2626
const empty = {};
27+
const contextValue = {
28+
mainTargetedIndex: 'index',
29+
};
30+
31+
describe('getSearchParameters', () => {
32+
it('sets facets * and maxValuesPerFacet by default', () => {
33+
const props = {
34+
contextValue,
35+
...connector.defaultProps,
36+
};
37+
const searchState = {};
38+
39+
const actual = connector.getSearchParameters(
40+
new SearchParameters(),
41+
props,
42+
searchState
43+
);
44+
45+
expect(actual).toEqual(
46+
new SearchParameters({
47+
facets: ['*'],
48+
maxValuesPerFacet: 20,
49+
})
50+
);
51+
});
2752

28-
describe('single index', () => {
29-
const contextValue = {
30-
mainTargetedIndex: 'index',
31-
};
53+
it('gets added onto existing parameters', () => {
54+
const props = {
55+
contextValue,
56+
...connector.defaultProps,
57+
};
58+
const searchState = {};
59+
60+
const actual = connector.getSearchParameters(
61+
new SearchParameters({
62+
facets: ['123'],
63+
maxValuesPerFacet: 1000,
64+
}),
65+
props,
66+
searchState
67+
);
68+
69+
expect(actual).toEqual(
70+
new SearchParameters({
71+
facets: ['123', '*'],
72+
maxValuesPerFacet: 1000,
73+
})
74+
);
75+
});
3276

77+
it('allows override of facets and maxValuesPerFacet', () => {
78+
const props = {
79+
contextValue,
80+
...connector.defaultProps,
81+
facets: ['lol'],
82+
maxValuesPerFacet: 1000,
83+
};
84+
const searchState = {};
85+
86+
const actual = connector.getSearchParameters(
87+
new SearchParameters(),
88+
props,
89+
searchState
90+
);
91+
92+
expect(actual).toEqual(
93+
new SearchParameters({
94+
facets: ['lol'],
95+
maxValuesPerFacet: 1000,
96+
})
97+
);
98+
});
99+
});
100+
101+
describe('single index', () => {
33102
const createSingleIndexSearchResults = (result = {}, state) => ({
34103
results: new SearchResults(new SearchParameters(state), [
35104
{
@@ -151,11 +220,143 @@ describe('connectDynamicWidgets', () => {
151220

152221
expect(actual.attributesToRender).toEqual(['one', 'two']);
153222
});
223+
224+
it('fails when a non-star facet is given', () => {
225+
const props = {
226+
contextValue,
227+
...connector.defaultProps,
228+
facets: ['lol'],
229+
};
230+
const searchState = {};
231+
const searchResults = createSingleIndexSearchResults({});
232+
233+
expect(() =>
234+
connector.getProvidedProps(props, searchState, searchResults)
235+
).toThrowErrorMatchingInlineSnapshot(
236+
`"The \`facets\` prop only accepts [] or [\\"*\\"], you passed [\\"lol\\"]"`
237+
);
238+
});
239+
240+
it('fails when a multiple star facets are given', () => {
241+
const props = {
242+
contextValue,
243+
...connector.defaultProps,
244+
facets: ['*', '*'],
245+
};
246+
const searchState = {};
247+
const searchResults = createSingleIndexSearchResults({});
248+
249+
expect(() =>
250+
connector.getProvidedProps(props, searchState, searchResults)
251+
).toThrowErrorMatchingInlineSnapshot(
252+
`"The \`facets\` prop only accepts [] or [\\"*\\"], you passed [\\"*\\",\\"*\\"]"`
253+
);
254+
});
255+
256+
it('does not fail when only star facet is given', () => {
257+
const props = {
258+
contextValue,
259+
...connector.defaultProps,
260+
facets: ['*'],
261+
};
262+
const searchState = {};
263+
const searchResults = createSingleIndexSearchResults({});
264+
265+
expect(() =>
266+
connector.getProvidedProps(props, searchState, searchResults)
267+
).not.toThrow();
268+
});
269+
270+
it('does not fail when no facet is given', () => {
271+
const props = {
272+
contextValue,
273+
...connector.defaultProps,
274+
facets: [],
275+
};
276+
const searchState = {};
277+
const searchResults = createSingleIndexSearchResults({});
278+
279+
expect(() =>
280+
connector.getProvidedProps(props, searchState, searchResults)
281+
).not.toThrow();
282+
});
283+
284+
it('warns if maxValuesPerFacet is lower than set by another widget', () => {
285+
const spy = jest.spyOn(console, 'warn');
286+
const props = {
287+
contextValue,
288+
...connector.defaultProps,
289+
};
290+
const searchState = {};
291+
const searchResults = createSingleIndexSearchResults(
292+
{},
293+
new SearchParameters({ maxValuesPerFacet: 100 })
294+
);
295+
296+
connector.getProvidedProps(props, searchState, searchResults);
297+
298+
expect(spy).toHaveBeenCalledWith(
299+
'The maxValuesPerFacet set by dynamic widgets (20) is smaller than one of the limits set by a widget (100). This causes a mismatch in query parameters and thus an extra network request when that widget is mounted.'
300+
);
301+
});
302+
303+
it('warns if >20 facets are displayed due to implicit *', () => {
304+
const spy = jest.spyOn(console, 'warn');
305+
const props = {
306+
contextValue,
307+
transformItems: (_items, { results }) =>
308+
results.userData[0].MOCK_FACET_ORDER,
309+
};
310+
const searchState = {};
311+
const searchResults = createSingleIndexSearchResults({
312+
userData: [
313+
{
314+
MOCK_FACET_ORDER: Array.from(
315+
{ length: 21 },
316+
(_, i) => `item${i}`
317+
),
318+
},
319+
],
320+
});
321+
322+
const actual = connector.getProvidedProps(
323+
props,
324+
searchState,
325+
searchResults
326+
);
327+
328+
expect(spy).toHaveBeenCalledWith(
329+
'More than 20 facets are requested to be displayed without explicitly setting which facets to retrieve. This could have a performance impact. Set "facets" to [] to do two smaller network requests, or explicitly to [\'*\'] to avoid this warning.'
330+
);
331+
332+
expect(actual.attributesToRender).toEqual([
333+
'item0',
334+
'item1',
335+
'item2',
336+
'item3',
337+
'item4',
338+
'item5',
339+
'item6',
340+
'item7',
341+
'item8',
342+
'item9',
343+
'item10',
344+
'item11',
345+
'item12',
346+
'item13',
347+
'item14',
348+
'item15',
349+
'item16',
350+
'item17',
351+
'item18',
352+
'item19',
353+
'item20',
354+
]);
355+
});
154356
});
155357
});
156358

157359
describe('multi index', () => {
158-
const contextValue = { mainTargetedIndex: 'first' };
159360
const indexContextValue = { targetedIndex: 'second' };
160361

161362
const createMultiIndexSearchState = (state = {}) => ({

‎packages/react-instantsearch-core/src/connectors/connectDynamicWidgets.ts‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,22 @@
1+
import type { SearchParameters } from 'algoliasearch-helper';
12
import PropTypes from 'prop-types';
23
import createConnector from '../core/createConnector';
34
import { getResults } from '../core/indexUtils';
45

6+
const MAX_WILDCARD_FACETS = 20;
7+
58
export default createConnector({
69
displayName: 'AlgoliaDynamicWidgets',
710

811
defaultProps: {
912
transformItems: (items) => items,
13+
maxValuesPerFacet: 20,
1014
},
1115

1216
propTypes: {
1317
transformItems: PropTypes.func,
18+
facets: PropTypes.arrayOf(PropTypes.string),
19+
maxValuesPerFacet: PropTypes.number,
1420
},
1521

1622
getProvidedProps(props, _searchState, searchResults) {
@@ -19,6 +25,21 @@ export default createConnector({
1925
multiIndexContext: props.indexContextValue,
2026
});
2127

28+
if (
29+
props.facets &&
30+
!(
31+
Array.isArray(props.facets) &&
32+
props.facets.length <= 1 &&
33+
(props.facets[0] === '*' || props.facets[0] === undefined)
34+
)
35+
) {
36+
throw new Error(
37+
`The \`facets\` prop only accepts [] or ["*"], you passed ${JSON.stringify(
38+
props.facets
39+
)}`
40+
);
41+
}
42+
2243
if (!results) {
2344
return { attributesToRender: [] };
2445
}
@@ -30,8 +51,36 @@ export default createConnector({
3051
results.renderingContent.facetOrdering.facets.order) ||
3152
[];
3253

54+
const attributesToRender = props.transformItems(facetOrder, { results });
55+
56+
if (attributesToRender.length > MAX_WILDCARD_FACETS && !props.facets) {
57+
// eslint-disable-next-line no-console
58+
console.warn(
59+
`More than ${MAX_WILDCARD_FACETS} facets are requested to be displayed without explicitly setting which facets to retrieve. This could have a performance impact. Set "facets" to [] to do two smaller network requests, or explicitly to ['*'] to avoid this warning.`
60+
);
61+
}
62+
63+
if (props.maxValuesPerFacet < results._state.maxValuesPerFacet) {
64+
// eslint-disable-next-line no-console
65+
console.warn(
66+
`The maxValuesPerFacet set by dynamic widgets (${props.maxValuesPerFacet}) is smaller than one of the limits set by a widget (${results._state.maxValuesPerFacet}). This causes a mismatch in query parameters and thus an extra network request when that widget is mounted.`
67+
);
68+
}
69+
3370
return {
34-
attributesToRender: props.transformItems(facetOrder, { results }),
71+
attributesToRender,
3572
};
3673
},
74+
75+
getSearchParameters(searchParameters, props) {
76+
return (props.facets || ['*']).reduce(
77+
(acc: SearchParameters, curr: string) => acc.addFacet(curr),
78+
searchParameters.setQueryParameters({
79+
maxValuesPerFacet: Math.max(
80+
props.maxValuesPerFacet || 0,
81+
searchParameters.maxValuesPerFacet || 0
82+
),
83+
})
84+
);
85+
},
3786
});

0 commit comments

Comments
 (0)