fix(removeXFacet): make sure this fully removes empty arrays (#743)

* fix(removeXFacet): make sure this fully removes empty arrays

This does not have an averse on the toggle methods, which should keep empty arrays.

required for algolia/instantsearch#4025

* test: add tests for fully empty removal

src/SearchParameters/RefinementList.js

@@ -99,9 +99,6 @@ var lib = {
       }
       return {};
     } else if (typeof attribute === 'string') {
-      if (!(refinementList[attribute] && refinementList[attribute].length > 0)) {
-        return refinementList;
-      }
       return omit(refinementList, attribute);
     } else if (typeof attribute === 'function') {
       var hasChanged = false;

src/SearchParameters/index.js

@@ -369,19 +369,36 @@ SearchParameters.prototype = {
    * @return {SearchParameters}
    */
   clearRefinements: function clearRefinements(attribute) {
-    var clear = RefinementList.clearRefinement;
     var patch = {
       numericRefinements: this._clearNumericRefinements(attribute),
-      facetsRefinements: clear(this.facetsRefinements, attribute, 'conjunctiveFacet'),
-      facetsExcludes: clear(this.facetsExcludes, attribute, 'exclude'),
-      disjunctiveFacetsRefinements: clear(this.disjunctiveFacetsRefinements, attribute, 'disjunctiveFacet'),
-      hierarchicalFacetsRefinements: clear(this.hierarchicalFacetsRefinements, attribute, 'hierarchicalFacet')
+      facetsRefinements: RefinementList.clearRefinement(
+        this.facetsRefinements,
+        attribute,
+        'conjunctiveFacet'
+      ),
+      facetsExcludes: RefinementList.clearRefinement(
+        this.facetsExcludes,
+        attribute,
+        'exclude'
+      ),
+      disjunctiveFacetsRefinements: RefinementList.clearRefinement(
+        this.disjunctiveFacetsRefinements,
+        attribute,
+        'disjunctiveFacet'
+      ),
+      hierarchicalFacetsRefinements: RefinementList.clearRefinement(
+        this.hierarchicalFacetsRefinements,
+        attribute,
+        'hierarchicalFacet'
+      )
     };
-    if (patch.numericRefinements === this.numericRefinements &&
-        patch.facetsRefinements === this.facetsRefinements &&
-        patch.facetsExcludes === this.facetsExcludes &&
-        patch.disjunctiveFacetsRefinements === this.disjunctiveFacetsRefinements &&
-        patch.hierarchicalFacetsRefinements === this.hierarchicalFacetsRefinements) {
+    if (
+      patch.numericRefinements === this.numericRefinements &&
+      patch.facetsRefinements === this.facetsRefinements &&
+      patch.facetsExcludes === this.facetsExcludes &&
+      patch.disjunctiveFacetsRefinements === this.disjunctiveFacetsRefinements &&
+      patch.hierarchicalFacetsRefinements === this.hierarchicalFacetsRefinements
+    ) {
       return this;
     }
     return this.setQueryParameters(patch);

test/spec/SearchParameters/RefinementList/clear.js

@@ -13,7 +13,7 @@ test('When removing refinements of a specific attribute, and there are no refine
     'attribute': ['test']
   };
 
-  expect(clear(initialRefinementList, 'notThisAttribute')).toBe(initialRefinementList);
+  expect(clear(initialRefinementList, 'notThisAttribute')).toEqual(initialRefinementList);
 });
 
 test('When removing numericRefinements using a function, and there are no changes', function() {
@@ -30,3 +30,10 @@ test('When removing numericRefinements using a function, and there are no change
   expect(clear(initialRefinementList, clearUndefinedValue, 'facet')).toBe(initialRefinementList);
 });
 
+test('calling clear on a empty refinement removes it', function() {
+  var initialRefinementList = {
+    'attribute': []
+  };
+
+  expect(clear(initialRefinementList, 'attribute')).toEqual({});
+});

test/spec/SearchParameters/removeXFacet.js

@@ -0,0 +1,162 @@
+'use strict';
+
+var SearchParameters = require('../../../src/SearchParameters');
+
+describe('removeDisjunctiveFacet', function() {
+  test('removeDisjunctiveFacet(attribute), multiple refinements', function() {
+    var state = new SearchParameters({
+      disjunctiveFacets: ['attribute', 'other'],
+      disjunctiveFacetsRefinements: {
+        attribute: ['value'],
+        other: ['value']
+      }
+    });
+
+    expect(state.removeDisjunctiveFacet('attribute')).toEqual(
+      new SearchParameters({
+        disjunctiveFacets: ['other'],
+        disjunctiveFacetsRefinements: {
+          other: ['value']
+        }
+      })
+    );
+  });
+
+  test('removeDisjunctiveFacet(attribute), empty refinements', function() {
+    var state = new SearchParameters({
+      disjunctiveFacets: ['attribute'],
+      disjunctiveFacetsRefinements: {
+        attribute: []
+      }
+    });
+
+    expect(state.removeDisjunctiveFacet('attribute')).toEqual(
+      new SearchParameters()
+    );
+  });
+
+  test('removeDisjunctiveFacet(attribute), no refinements', function() {
+    var state = new SearchParameters({
+      disjunctiveFacets: ['attribute']
+    });
+
+    expect(state.removeDisjunctiveFacet('attribute')).toEqual(
+      new SearchParameters()
+    );
+  });
+
+  test('removeDisjunctiveFacet(attribute), empty', function() {
+    var state = new SearchParameters();
+
+    expect(state.removeDisjunctiveFacet('attribute')).toEqual(
+      new SearchParameters()
+    );
+  });
+});
+
+describe('removeFacet', function() {
+  test('removeFacet(attribute), multiple refinements', function() {
+    var state = new SearchParameters({
+      facets: ['attribute', 'other'],
+      facetsRefinements: {
+        attribute: ['value'],
+        other: ['value']
+      }
+    });
+
+    expect(state.removeFacet('attribute')).toEqual(
+      new SearchParameters({
+        facets: ['other'],
+        facetsRefinements: {
+          other: ['value']
+        }
+      })
+    );
+  });
+
+  test('removeFacet(attribute), empty refinements', function() {
+    var state = new SearchParameters({
+      facets: ['attribute'],
+      facetsRefinements: {
+        attribute: []
+      }
+    });
+
+    expect(state.removeFacet('attribute')).toEqual(
+      new SearchParameters()
+    );
+  });
+
+  test('removeFacet(attribute), no refinements', function() {
+    var state = new SearchParameters({
+      facets: ['attribute']
+    });
+
+    expect(state.removeFacet('attribute')).toEqual(
+      new SearchParameters()
+    );
+  });
+
+  test('removeFacet(attribute), empty', function() {
+    var state = new SearchParameters();
+
+    expect(state.removeFacet('attribute')).toEqual(
+      new SearchParameters()
+    );
+  });
+});
+
+describe('removeHierarchicalFacet', function() {
+  test('removeHierarchicalFacet(attribute), multiple refinements', function() {
+    var state = new SearchParameters({
+      hierarchicalFacets: [
+        {name: 'attribute', attributes: ['zip', 'zop']},
+        {name: 'other', attributes: ['zap', 'zep']}
+      ],
+      hierarchicalFacetsRefinements: {
+        attribute: ['value'],
+        other: ['value']
+      }
+    });
+
+    expect(state.removeHierarchicalFacet('attribute')).toEqual(
+      new SearchParameters({
+        hierarchicalFacets: [{name: 'other', attributes: ['zap', 'zep']}],
+        hierarchicalFacetsRefinements: {
+          other: ['value']
+        }
+      })
+    );
+  });
+
+  test('removeHierarchicalFacet(attribute), empty refinements', function() {
+    var state = new SearchParameters({
+      hierarchicalFacets: [{name: 'attribute', attributes: ['zip', 'zop']}],
+      hierarchicalFacetsRefinements: {
+        attribute: []
+      }
+    });
+
+    expect(state.removeHierarchicalFacet('attribute')).toEqual(
+      new SearchParameters()
+    );
+  });
+
+  test('removeHierarchicalFacet(attribute), no refinements', function() {
+    var state = new SearchParameters({
+      hierarchicalFacets: [{name: 'attribute', attributes: ['zip', 'zop']}]
+    });
+
+    expect(state.removeHierarchicalFacet('attribute')).toEqual(
+      new SearchParameters()
+    );
+  });
+
+  test('removeHierarchicalFacet(attribute), empty', function() {
+    var state = new SearchParameters();
+
+    expect(state.removeHierarchicalFacet('attribute')).toEqual(
+      new SearchParameters()
+    );
+  });
+});

test/spec/algoliasearch.helper/clears.js

@@ -226,10 +226,10 @@ test('Clearing with no effect should not update the state, if used with an unkno
   // This operation should not update the reference to the state
   helper.clearRefinements('unknown');
 
-  expect(helper.state.numericRefinements).toBe(initialState.numericRefinements);
-  expect(helper.state.facetsRefinements).toBe(initialState.facetsRefinements);
-  expect(helper.state.disjunctiveFacetsRefinements).toBe(initialState.disjunctiveFacetsRefinements);
-  expect(helper.state.hierarchicalFacetsRefinements).toBe(initialState.hierarchicalFacetsRefinements);
+  expect(helper.state.numericRefinements).toEqual(initialState.numericRefinements);
+  expect(helper.state.facetsRefinements).toEqual(initialState.facetsRefinements);
+  expect(helper.state.disjunctiveFacetsRefinements).toEqual(initialState.disjunctiveFacetsRefinements);
+  expect(helper.state.hierarchicalFacetsRefinements).toEqual(initialState.hierarchicalFacetsRefinements);
 
-  expect(helper.state).toBe(initialState);
+  expect(helper.state).toEqual(initialState);
 });