fix(enhanceConfiguration): deduplicate the hierarchicalFacets (#3966)

* feat(utils): add findIndex

* fix(enhanceConfiguration): merge hierarchicalFacets

src/lib/InstantSearch.js

@@ -7,9 +7,10 @@ import version from './version';
 import createHelpers from './createHelpers';
 import {
   createDocumentationMessageGenerator,
-  noop,
+  findIndex,
   isPlainObject,
   mergeDeep,
+  noop,
 } from './utils';
 
 const withUsage = createDocumentationMessageGenerator({
@@ -440,7 +441,46 @@ export function enhanceConfiguration(configuration, widgetDefinition) {
   // Get the relevant partial configuration asked by the widget
   const partialConfiguration = widgetDefinition.getConfiguration(configuration);
 
-  return mergeDeep(configuration, partialConfiguration);
+  if (!partialConfiguration) {
+    return configuration;
+  }
+
+  if (!partialConfiguration.hierarchicalFacets) {
+    return mergeDeep(configuration, partialConfiguration);
+  }
+
+  const {
+    hierarchicalFacets,
+    ...partialWithoutHierarchcialFacets
+  } = partialConfiguration;
+
+  // The `mergeDeep` function uses a `uniq` function under the hood, but the
+  // implementation does not support arrays of objects (we also had the issue
+  // with the Lodash version). The `hierarchicalFacets` attribute is an array
+  // of objects, which means that this attribute is never deduplicated. It
+  // becomes problematic when widgets are frequently added/removed, since the
+  // function `enhanceConfiguration` is called at each operation.
+  // https://github.com/algolia/instantsearch.js/issues/3278
+  const configurationWithHierarchicalFacets = {
+    ...configuration,
+    hierarchicalFacets: hierarchicalFacets.reduce((facets, facet) => {
+      const index = findIndex(facets, _ => _.name === facet.name);
+
+      if (index === -1) {
+        return facets.concat(facet);
+      }
+
+      const nextFacets = facets.slice();
+      nextFacets.splice(index, 1, facet);
+
+      return nextFacets;
+    }, configuration.hierarchicalFacets || []),
+  };
+
+  return mergeDeep(
+    configurationWithHierarchicalFacets,
+    partialWithoutHierarchcialFacets
+  );
 }
 
 export default InstantSearch;

src/lib/__tests__/enhanceConfiguration-test.js

@@ -48,7 +48,7 @@ describe('enhanceConfiguration', () => {
     expect(output.analytics).toBe(true);
   });
 
-  it('should union array', () => {
+  it('should deduplicate primitive array', () => {
     {
       const actualConfiguration = { refinements: ['foo'] };
       const widget = createWidget({ refinements: ['foo', 'bar'] });
@@ -83,4 +83,139 @@ describe('enhanceConfiguration', () => {
       refinements: { lvl1: ['foo', 'bar'], lvl2: true },
     });
   });
+
+  it('should add `hierarchicalFacets`', () => {
+    const actualConfiguration = {};
+
+    const widget = createWidget({
+      hierarchicalFacets: [
+        {
+          name: 'categories',
+          attributes: [
+            'categories.lvl0',
+            'categories.lvl1',
+            'categories.lvl2',
+            'categories.lvl3',
+          ],
+        },
+      ],
+    });
+
+    const output = enhanceConfiguration(actualConfiguration, widget);
+
+    expect(output).toEqual({
+      hierarchicalFacets: [
+        {
+          name: 'categories',
+          attributes: [
+            'categories.lvl0',
+            'categories.lvl1',
+            'categories.lvl2',
+            'categories.lvl3',
+          ],
+        },
+      ],
+    });
+  });
+
+  it('should add multiple `hierarchicalFacets`', () => {
+    const actualConfiguration = {
+      hierarchicalFacets: [
+        {
+          name: 'countries',
+          attributes: [
+            'countries.lvl0',
+            'countries.lvl1',
+            'countries.lvl2',
+            'countries.lvl3',
+          ],
+        },
+      ],
+    };
+
+    const widget = createWidget({
+      hierarchicalFacets: [
+        {
+          name: 'categories',
+          attributes: [
+            'categories.lvl0',
+            'categories.lvl1',
+            'categories.lvl2',
+            'categories.lvl3',
+          ],
+        },
+      ],
+    });
+
+    const output = enhanceConfiguration(actualConfiguration, widget);
+
+    expect(output).toEqual({
+      hierarchicalFacets: [
+        {
+          name: 'countries',
+          attributes: [
+            'countries.lvl0',
+            'countries.lvl1',
+            'countries.lvl2',
+            'countries.lvl3',
+          ],
+        },
+        {
+          name: 'categories',
+          attributes: [
+            'categories.lvl0',
+            'categories.lvl1',
+            'categories.lvl2',
+            'categories.lvl3',
+          ],
+        },
+      ],
+    });
+  });
+
+  it('should deduplicate `hierarchicalFacets` with same name', () => {
+    const actualConfiguration = {
+      hierarchicalFacets: [
+        {
+          name: 'categories',
+          attributes: [
+            'categories.lvl0',
+            'categories.lvl1',
+            'categories.lvl2',
+            'categories.lvl3',
+          ],
+        },
+      ],
+    };
+
+    const widget = createWidget({
+      hierarchicalFacets: [
+        {
+          name: 'categories',
+          attributes: [
+            'categories.lvl0',
+            'categories.lvl1',
+            'categories.lvl2',
+            'categories.lvl3',
+          ],
+        },
+      ],
+    });
+
+    const output = enhanceConfiguration(actualConfiguration, widget);
+
+    expect(output).toEqual({
+      hierarchicalFacets: [
+        {
+          name: 'categories',
+          attributes: [
+            'categories.lvl0',
+            'categories.lvl1',
+            'categories.lvl2',
+            'categories.lvl3',
+          ],
+        },
+      ],
+    });
+  });
 });

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

@@ -0,0 +1,33 @@
+import findIndex from '../findIndex';
+
+describe('findIndex', () => {
+  describe('with polyfill', () => {
+    test('with empty array', () => {
+      const items = [];
+      const actual = findIndex(items, item => item === 'hello');
+
+      expect(actual).toEqual(-1);
+    });
+
+    test('with unknown item in array', () => {
+      const items = ['hey'];
+      const actual = findIndex(items, item => item === 'hello');
+
+      expect(actual).toEqual(-1);
+    });
+
+    test('with an array of strings', () => {
+      const items = ['hello', 'goodbye'];
+      const actual = findIndex(items, item => item === 'hello');
+
+      expect(actual).toEqual(0);
+    });
+
+    test('with an array of objects', () => {
+      const items = [{ name: 'John' }, { name: 'Jane' }];
+      const actual = findIndex(items, item => item.name === 'John');
+
+      expect(actual).toEqual(0);
+    });
+  });
+});

src/lib/utils/findIndex.ts

@@ -0,0 +1,23 @@
+// We aren't using the native `Array.prototype.findIndex` because the refactor away from Lodash is not
+// published as a major version.
+// Relying on the `findIndex` polyfill on user-land, which before was only required for niche use-cases,
+// was decided as too risky.
+// @MAJOR Replace with the native `Array.prototype.findIndex` method
+// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/findIndex
+function findIndex<TItem>(
+  array: TItem[],
+  comparator: (value: TItem) => boolean
+): number {
+  if (!Array.isArray(array)) {
+    return -1;
+  }
+
+  for (let i = 0; i < array.length; i++) {
+    if (comparator(array[i])) {
+      return i;
+    }
+  }
+  return -1;
+}
+
+export default findIndex;

src/lib/utils/index.ts

@@ -18,6 +18,7 @@ export { default as range } from './range';
 export { default as isEqual } from './isEqual';
 export { default as escape } from './escape';
 export { default as find } from './find';
+export { default as findIndex } from './findIndex';
 export { default as mergeDeep } from './mergeDeep';
 export { warning, deprecate } from './logger';
 export {