Skip to content

Commit baf8a35

Browse files
samousssamouss
authored
fix(enhanceConfiguration): deduplicate the hierarchicalFacets (#3966)
* feat(utils): add findIndex * fix(enhanceConfiguration): merge hierarchicalFacets
1 parent 5ee79fa commit baf8a35

File tree

5 files changed

+235
-3
lines changed

5 files changed

+235
-3
lines changed

‎src/lib/InstantSearch.js‎

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@ import version from './version';
77
import createHelpers from './createHelpers';
88
import {
99
createDocumentationMessageGenerator,
10-
noop,
10+
findIndex,
1111
isPlainObject,
1212
mergeDeep,
13+
noop,
1314
} from './utils';
1415

1516
const withUsage = createDocumentationMessageGenerator({
@@ -440,7 +441,46 @@ export function enhanceConfiguration(configuration, widgetDefinition) {
440441
// Get the relevant partial configuration asked by the widget
441442
const partialConfiguration = widgetDefinition.getConfiguration(configuration);
442443

443-
return mergeDeep(configuration, partialConfiguration);
444+
if (!partialConfiguration) {
445+
return configuration;
446+
}
447+
448+
if (!partialConfiguration.hierarchicalFacets) {
449+
return mergeDeep(configuration, partialConfiguration);
450+
}
451+
452+
const {
453+
hierarchicalFacets,
454+
...partialWithoutHierarchcialFacets
455+
} = partialConfiguration;
456+
457+
// The `mergeDeep` function uses a `uniq` function under the hood, but the
458+
// implementation does not support arrays of objects (we also had the issue
459+
// with the Lodash version). The `hierarchicalFacets` attribute is an array
460+
// of objects, which means that this attribute is never deduplicated. It
461+
// becomes problematic when widgets are frequently added/removed, since the
462+
// function `enhanceConfiguration` is called at each operation.
463+
// https://github.com/algolia/instantsearch.js/issues/3278
464+
const configurationWithHierarchicalFacets = {
465+
...configuration,
466+
hierarchicalFacets: hierarchicalFacets.reduce((facets, facet) => {
467+
const index = findIndex(facets, _ => _.name === facet.name);
468+
469+
if (index === -1) {
470+
return facets.concat(facet);
471+
}
472+
473+
const nextFacets = facets.slice();
474+
nextFacets.splice(index, 1, facet);
475+
476+
return nextFacets;
477+
}, configuration.hierarchicalFacets || []),
478+
};
479+
480+
return mergeDeep(
481+
configurationWithHierarchicalFacets,
482+
partialWithoutHierarchcialFacets
483+
);
444484
}
445485

446486
export default InstantSearch;

‎src/lib/__tests__/enhanceConfiguration-test.js‎

Lines changed: 136 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ describe('enhanceConfiguration', () => {
4848
expect(output.analytics).toBe(true);
4949
});
5050

51-
it('should union array', () => {
51+
it('should deduplicate primitive array', () => {
5252
{
5353
const actualConfiguration = { refinements: ['foo'] };
5454
const widget = createWidget({ refinements: ['foo', 'bar'] });
@@ -83,4 +83,139 @@ describe('enhanceConfiguration', () => {
8383
refinements: { lvl1: ['foo', 'bar'], lvl2: true },
8484
});
8585
});
86+
87+
it('should add `hierarchicalFacets`', () => {
88+
const actualConfiguration = {};
89+
90+
const widget = createWidget({
91+
hierarchicalFacets: [
92+
{
93+
name: 'categories',
94+
attributes: [
95+
'categories.lvl0',
96+
'categories.lvl1',
97+
'categories.lvl2',
98+
'categories.lvl3',
99+
],
100+
},
101+
],
102+
});
103+
104+
const output = enhanceConfiguration(actualConfiguration, widget);
105+
106+
expect(output).toEqual({
107+
hierarchicalFacets: [
108+
{
109+
name: 'categories',
110+
attributes: [
111+
'categories.lvl0',
112+
'categories.lvl1',
113+
'categories.lvl2',
114+
'categories.lvl3',
115+
],
116+
},
117+
],
118+
});
119+
});
120+
121+
it('should add multiple `hierarchicalFacets`', () => {
122+
const actualConfiguration = {
123+
hierarchicalFacets: [
124+
{
125+
name: 'countries',
126+
attributes: [
127+
'countries.lvl0',
128+
'countries.lvl1',
129+
'countries.lvl2',
130+
'countries.lvl3',
131+
],
132+
},
133+
],
134+
};
135+
136+
const widget = createWidget({
137+
hierarchicalFacets: [
138+
{
139+
name: 'categories',
140+
attributes: [
141+
'categories.lvl0',
142+
'categories.lvl1',
143+
'categories.lvl2',
144+
'categories.lvl3',
145+
],
146+
},
147+
],
148+
});
149+
150+
const output = enhanceConfiguration(actualConfiguration, widget);
151+
152+
expect(output).toEqual({
153+
hierarchicalFacets: [
154+
{
155+
name: 'countries',
156+
attributes: [
157+
'countries.lvl0',
158+
'countries.lvl1',
159+
'countries.lvl2',
160+
'countries.lvl3',
161+
],
162+
},
163+
{
164+
name: 'categories',
165+
attributes: [
166+
'categories.lvl0',
167+
'categories.lvl1',
168+
'categories.lvl2',
169+
'categories.lvl3',
170+
],
171+
},
172+
],
173+
});
174+
});
175+
176+
it('should deduplicate `hierarchicalFacets` with same name', () => {
177+
const actualConfiguration = {
178+
hierarchicalFacets: [
179+
{
180+
name: 'categories',
181+
attributes: [
182+
'categories.lvl0',
183+
'categories.lvl1',
184+
'categories.lvl2',
185+
'categories.lvl3',
186+
],
187+
},
188+
],
189+
};
190+
191+
const widget = createWidget({
192+
hierarchicalFacets: [
193+
{
194+
name: 'categories',
195+
attributes: [
196+
'categories.lvl0',
197+
'categories.lvl1',
198+
'categories.lvl2',
199+
'categories.lvl3',
200+
],
201+
},
202+
],
203+
});
204+
205+
const output = enhanceConfiguration(actualConfiguration, widget);
206+
207+
expect(output).toEqual({
208+
hierarchicalFacets: [
209+
{
210+
name: 'categories',
211+
attributes: [
212+
'categories.lvl0',
213+
'categories.lvl1',
214+
'categories.lvl2',
215+
'categories.lvl3',
216+
],
217+
},
218+
],
219+
});
220+
});
86221
});

‎src/lib/utils/__tests__/findIndex-test.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import findIndex from '../findIndex';
2+
3+
describe('findIndex', () => {
4+
describe('with polyfill', () => {
5+
test('with empty array', () => {
6+
const items = [];
7+
const actual = findIndex(items, item => item === 'hello');
8+
9+
expect(actual).toEqual(-1);
10+
});
11+
12+
test('with unknown item in array', () => {
13+
const items = ['hey'];
14+
const actual = findIndex(items, item => item === 'hello');
15+
16+
expect(actual).toEqual(-1);
17+
});
18+
19+
test('with an array of strings', () => {
20+
const items = ['hello', 'goodbye'];
21+
const actual = findIndex(items, item => item === 'hello');
22+
23+
expect(actual).toEqual(0);
24+
});
25+
26+
test('with an array of objects', () => {
27+
const items = [{ name: 'John' }, { name: 'Jane' }];
28+
const actual = findIndex(items, item => item.name === 'John');
29+
30+
expect(actual).toEqual(0);
31+
});
32+
});
33+
});

‎src/lib/utils/findIndex.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// We aren't using the native `Array.prototype.findIndex` because the refactor away from Lodash is not
2+
// published as a major version.
3+
// Relying on the `findIndex` polyfill on user-land, which before was only required for niche use-cases,
4+
// was decided as too risky.
5+
// @MAJOR Replace with the native `Array.prototype.findIndex` method
6+
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/findIndex
7+
function findIndex<TItem>(
8+
array: TItem[],
9+
comparator: (value: TItem) => boolean
10+
): number {
11+
if (!Array.isArray(array)) {
12+
return -1;
13+
}
14+
15+
for (let i = 0; i < array.length; i++) {
16+
if (comparator(array[i])) {
17+
return i;
18+
}
19+
}
20+
return -1;
21+
}
22+
23+
export default findIndex;

‎src/lib/utils/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ export { default as range } from './range';
1818
export { default as isEqual } from './isEqual';
1919
export { default as escape } from './escape';
2020
export { default as find } from './find';
21+
export { default as findIndex } from './findIndex';
2122
export { default as mergeDeep } from './mergeDeep';
2223
export { warning, deprecate } from './logger';
2324
export {

0 commit comments

Comments
 (0)