feat(SearchParameters): avoid undefined values (#703)

* refactor(SearchParameters): omit undefined values on mutation

* refactor(SearchParameters): remove unused method mutateMe

* test(setQueryParameters): simplify assertions

* refactor(SearchParameters): omit undefined values on creation

* refactor(SearchParameters): invert condition for known keys

src/SearchParameters/index.js

@@ -197,7 +197,10 @@ function SearchParameters(newParameters) {
 
   var self = this;
   Object.keys(params).forEach(function(paramName) {
-    if (SearchParameters.PARAMETERS.indexOf(paramName) === -1) {
+    var isKeyKnown = SearchParameters.PARAMETERS.indexOf(paramName) !== -1;
+    var isValueDefined = params[paramName] !== undefined;
+
+    if (!isKeyKnown && isValueDefined) {
       self[paramName] = params[paramName];
     }
   });
@@ -1358,15 +1361,32 @@ SearchParameters.prototype = {
       throw error;
     }
 
-    var parsedParams = SearchParameters._parseNumbers(params);
+    var self = this;
+    var nextWithNumbers = SearchParameters._parseNumbers(params);
+    var previousPlainObject = Object.keys(this).reduce(function(acc, key) {
+      acc[key] = self[key];
+      return acc;
+    }, {});
+
+    var nextPlainObject = Object.keys(nextWithNumbers).reduce(
+      function(previous, key) {
+        var isPreviousValueDefined = previous[key] !== undefined;
+        var isNextValueDefined = nextWithNumbers[key] !== undefined;
+
+        if (isPreviousValueDefined && !isNextValueDefined) {
+          return omit(previous, [key]);
+        }
 
-    return this.mutateMe(function mergeWith(newInstance) {
-      Object.keys(params).forEach(function(k) {
-        newInstance[k] = parsedParams[k];
-      });
+        if (isNextValueDefined) {
+          previous[key] = nextWithNumbers[key];
+        }
 
-      return newInstance;
-    });
+        return previous;
+      },
+      previousPlainObject
+    );
+
+    return new this.constructor(nextPlainObject);
   },
 
   /**
@@ -1393,20 +1413,6 @@ SearchParameters.prototype = {
   filter: function(filters) {
     return filterState(this, filters);
   },
-  /**
-   * Helper function to make it easier to build new instances from a mutating
-   * function
-   * @private
-   * @param {function} fn newMutableState -> previousState -> () function that will
-   * change the value of the newMutable to the desired state
-   * @return {SearchParameters} a new instance with the specified modifications applied
-   */
-  mutateMe: function mutateMe(fn) {
-    var newState = new this.constructor(this);
-
-    fn(newState, this);
-    return newState;
-  },
 
   /**
    * Helper function to get the hierarchicalFacet separator or the default one (`>`)

test/spec/SearchParameters/constructorFn.js

@@ -51,6 +51,15 @@ test('Constructor should accept an object with unknown keys', function() {
   });
 });
 
+test('Constructor should ignore keys with undefined values', function() {
+  var state = new SearchParameters({
+    query: '',
+    page: undefined
+  });
+
+  expect(state).not.toHaveProperty('page');
+});
+
 test('Factory should accept an object with known keys', function() {
   var legitConfig = {
     'query': '',
@@ -74,7 +83,7 @@ test('Factory should accept an object with known keys', function() {
   });
 });
 
-test('Constructor should accept an object with unknown keys', function() {
+test('Factory should accept an object with unknown keys', function() {
   var betaConfig = {
     'query': '',
     'disjunctiveFacets': [
@@ -98,3 +107,12 @@ test('Constructor should accept an object with unknown keys', function() {
     expect(params[k]).toEqual(v);
   });
 });
+
+test('Factory should ignore keys with undefined values', function() {
+  var state = SearchParameters.make({
+    query: '',
+    page: undefined
+  });
+
+  expect(state).not.toHaveProperty('page');
+});

test/spec/SearchParameters/setQueryParameters.js

@@ -1,22 +1,21 @@
 'use strict';
 
-var forOwn = require('lodash/forOwn');
 var SearchParameters = require('../../../src/SearchParameters');
 
 test('setQueryParameters should return the same instance if the options is falsey', function() {
-  var originalSP = new SearchParameters({
+  var state = new SearchParameters({
     facets: ['a', 'b'],
     ignorePlurals: false,
     attributesToHighlight: ''
   });
 
-  expect(originalSP).toBe(originalSP.setQueryParameters());
-  expect(originalSP).toBe(originalSP.setQueryParameters(null));
-  expect(originalSP).toBe(originalSP.setQueryParameters(undefined));
+  expect(state).toBe(state.setQueryParameters());
+  expect(state).toBe(state.setQueryParameters(null));
+  expect(state).toBe(state.setQueryParameters(undefined));
 });
 
 test('setQueryParameters should be able to mix an actual state with a new set of parameters', function() {
-  var originalSP = new SearchParameters({
+  var state0 = new SearchParameters({
     facets: ['a', 'b'],
     ignorePlurals: false,
     attributesToHighlight: ''
@@ -27,12 +26,15 @@ test('setQueryParameters should be able to mix an actual state with a new set of
     attributesToHighlight: ['city', 'country'],
     replaceSynonymsInHighlight: true
   };
-  var newSP = originalSP.setQueryParameters(params);
 
-  expect(newSP.facets).toEqual(params.facets);
-  expect(newSP.attributesToHighlight).toEqual(newSP.attributesToHighlight);
-  expect(newSP.replaceSynonymsInHighlight).toBe(newSP.replaceSynonymsInHighlight);
-  expect(newSP.ignorePlurals).toBe(originalSP.ignorePlurals);
+  var state1 = state0.setQueryParameters(params);
+
+  expect(state1).toEqual(new SearchParameters({
+    facets: ['a', 'c'],
+    ignorePlurals: false,
+    attributesToHighlight: ['city', 'country'],
+    replaceSynonymsInHighlight: true
+  }));
 });
 
 test('setQueryParameters should add unknown properties', function() {
@@ -49,7 +51,41 @@ test('setQueryParameters should add unknown properties', function() {
 
   var state1 = state0.setQueryParameters(params);
 
-  forOwn(params, function(v, k) {
-    expect(state1[k]).toEqual(v);
+  expect(state1).toEqual(new SearchParameters({
+    facets: ['a', 'b'],
+    ignorePlurals: false,
+    attributesToHighlight: '',
+    unknow1: ['a', 'c'],
+    facet: ['city', 'country']
+  }));
+});
+
+test('setQueryParameters should ignore undefined parameters without previous values', function() {
+  var state0 = new SearchParameters({
+    aroundLatLng: '10,12'
+  });
+
+  var state1 = state0.setQueryParameters({
+    query: undefined,
+    page: undefined
+  });
+
+  expect(state1).not.toHaveProperty('query');
+  expect(state1).not.toHaveProperty('page');
+});
+
+test('setQueryParameters should omit defined parameters with next values of undefined', function() {
+  var state0 = new SearchParameters({
+    aroundLatLng: '10,12',
+    query: 'Apple',
+    page: 5
   });
+
+  var state1 = state0.setQueryParameters({
+    query: undefined,
+    page: undefined
+  });
+
+  expect(state1).not.toHaveProperty('query');
+  expect(state1).not.toHaveProperty('page');
 });