fix(numericRefinementList): Correctly apply active class

Fixes #1010

The `shouldComponentUpdate` of `RefinementList` was checking if the
`facetValues` from the props where actually changed before issuing
a new rendering. It turns out that in `numericRefinementList`, the
new `facetValues` was always equal to the already set one. More
precisely, the filters seemed to already be set in the props before
reaching `shouldComponentUpdate`.

Turns out that we were editing a variable in place instead of working
on a copy so the internal props where changing without the component
really noticing.

I updated the `render` method of the `numericRefinementList` to pass
a new array instead of working on the same copy over and over again.

I refactored the `shouldComponentUpdate` and `_toggleRefinement`
methods by splitting in several variables in my quest to find the
issue. Thought I might keep it that way.

I added an explicit regression test to make sure we are really not
touching existing variables.

Now the current and next props are really different, and the full
render is propagated and the correct classes applied.# Possible types : feat, fix, refactor, chore, style, perf, test, docs

Author: pixelastic

src/components/RefinementList/RefinementList.js

@@ -17,7 +17,10 @@ class RefinementList extends React.Component {
   }
 
   shouldComponentUpdate(nextProps, nextState) {
-    return nextState !== this.state || !isEqual(this.props.facetValues, nextProps.facetValues);
+    let isStateDifferent = nextState !== this.state;
+    let isFacetValuesDifferent = !isEqual(this.props.facetValues, nextProps.facetValues);
+    let shouldUpdate = isStateDifferent || isFacetValuesDifferent;
+    return shouldUpdate;
   }
 
   refine(facetValueToRefine, isRefined) {

src/widgets/numeric-refinement-list/__tests__/numeric-refinement-list-test.js

@@ -4,6 +4,7 @@ import React from 'react';
 import expect from 'expect';
 import sinon from 'sinon';
 import jsdom from 'jsdom-global';
+import cloneDeep from 'lodash/lang/cloneDeep';
 
 import expectJSX from 'expect-jsx';
 import numericRefinementList from '../numeric-refinement-list.js';
@@ -183,6 +184,28 @@ describe('numericRefinementList()', () => {
     expect(helper.search.calledOnce).toBe(true, 'search called once');
   });
 
+  it('does not alter the initial options when rendering', () => {
+    // Note: https://github.com/algolia/instantsearch.js/issues/1010
+    // Make sure we work on a copy of the initial facetValues when rendering,
+    // not directly editing it
+
+    // Given
+    let initialOptions = [{start: 0, end: 5, name: '1-5'}];
+    let initialOptionsClone = cloneDeep(initialOptions);
+    let testWidget = numericRefinementList({
+      container,
+      attributeName: 'price',
+      options: initialOptions
+    });
+
+    // When
+    testWidget.render({state, results, createURL});
+
+    // Then
+    expect(initialOptions).toEqual(initialOptionsClone);
+  });
+
+
   afterEach(() => {
     numericRefinementList.__ResetDependency__('ReactDOM');
     numericRefinementList.__ResetDependency__('autoHideContainerHOC');

src/widgets/numeric-refinement-list/numeric-refinement-list.js

@@ -93,15 +93,18 @@ function numericRefinementList({
         templates
       });
 
-      this._toggleRefinement = facetValue => helper
-        .setState(refine(helper.state, attributeName, options, facetValue))
-        .search();
+      this._toggleRefinement = (facetValue) => {
+        let refinedState = refine(helper.state, attributeName, options, facetValue);
+        helper.setState(refinedState).search();
+      };
     },
     render: function({results, state, createURL}) {
-      let facetValues = options.map(facetValue => {
-        facetValue.isRefined = isRefined(state, attributeName, facetValue);
-        facetValue.attributeName = attributeName;
-        return facetValue;
+      let facetValues = options.map((facetValue) => {
+        return {
+          ...facetValue,
+          isRefined: isRefined(state, attributeName, facetValue),
+          attributeName: attributeName
+        };
       });
 
       // Bind createURL to this specific attribute