perf(React, widgets): implement shouldComponentUpdate, reduce bind

changes: - stop computing static things in every render() call - avoid using bind and creating too much function references - leverage static properties (no bind) and use shouldComponentUpdate were relevant
algolia · Feb 11, 2016 · 5efaac1 · 5efaac1
1 parent b056554
commit 5efaac1
Show file tree

Hide file tree

Showing 50 changed files with 927 additions and 688 deletions.
diff --git a/src/components/ClearAll/ClearAll.js b/src/components/ClearAll/ClearAll.js
@@ -3,6 +3,15 @@ import Template from '../Template.js';
 import {isSpecialClick} from '../../lib/utils.js';
 
 class ClearAll extends React.Component {
+  componentWillMount() {
+    this.handleClick = this.handleClick.bind(this);
+  }
+
+  shouldComponentUpdate(nextProps) {
+    return this.props.url !== nextProps.url ||
+      this.props.hasRefinements !== nextProps.hasRefinements;
+  }
+
   handleClick(e) {
     if (isSpecialClick(e)) {
       // do not alter the default browser behavior
@@ -14,16 +23,15 @@ class ClearAll extends React.Component {
   }
 
   render() {
-    const className = this.props.cssClasses.link;
     const data = {
       hasRefinements: this.props.hasRefinements
     };
 
     return (
       <a
-        className={className}
+        className={this.props.cssClasses.link}
         href={this.props.url}
-        onClick={this.handleClick.bind(this)}
+        onClick={this.handleClick}
       >
         <Template
           data={data}

diff --git a/src/components/CurrentRefinedValues/CurrentRefinedValues.js b/src/components/CurrentRefinedValues/CurrentRefinedValues.js
@@ -5,8 +5,13 @@ import Template from '../Template.js';
 import {isSpecialClick} from '../../lib/utils.js';
 import map from 'lodash/collection/map';
 import cloneDeep from 'lodash/lang/cloneDeep';
+import {isEqual} from 'lodash';
 
 class CurrentRefinedValues extends React.Component {
+  shouldComponentUpdate(nextProps) {
+    return !isEqual(this.props.refinements, nextProps.refinements);
+  }
+
   _clearAllElement(position, requestedPosition) {
     if (requestedPosition !== position) {
       return undefined;

diff --git a/src/components/Hits.js b/src/components/Hits.js
@@ -3,7 +3,15 @@ import map from 'lodash/collection/map';
 
 import Template from './Template.js';
 
+import {isEqual} from 'lodash';
+
 class Hits extends React.Component {
+  shouldComponentUpdate(nextProps) {
+    return this.props.results.hits.length === 0 ||
+      this.props.results.hits.length !== nextProps.results.hits.length ||
+      !isEqual(this.props.results.hits, nextProps.results.hits);
+  }
+
   renderWithResults() {
     let renderedHits = map(this.props.results.hits, hit => {
       return (

diff --git a/src/components/Pagination/Pagination.js b/src/components/Pagination/Pagination.js
@@ -11,21 +11,10 @@ import cx from 'classnames';
 class Pagination extends React.Component {
   constructor(props) {
     super(defaultsDeep(props, Pagination.defaultProps));
-  }
-
-  handleClick(pageNumber, event) {
-    if (isSpecialClick(event)) {
-      // do not alter the default browser behavior
-      // if one special key is down
-      return;
-    }
-    event.preventDefault();
-    this.props.setCurrentPage(pageNumber);
+    this.handleClick = this.handleClick.bind(this);
   }
 
   pageLink({label, ariaLabel, pageNumber, additionalClassName = null, isDisabled = false, isActive = false, createURL}) {
-    let handleClick = this.handleClick.bind(this, pageNumber);
-
     let cssClasses = {
       item: cx(this.props.cssClasses.item, additionalClassName),
       link: cx(this.props.cssClasses.link)
@@ -42,9 +31,10 @@ class Pagination extends React.Component {
       <PaginationLink
         ariaLabel={ariaLabel}
         cssClasses={cssClasses}
-        handleClick={handleClick}
-        key={label}
+        handleClick={this.handleClick}
+        key={label + pageNumber}
         label={label}
+        pageNumber={pageNumber}
         url={url}
       />
     );
@@ -113,6 +103,16 @@ class Pagination extends React.Component {
     return pages;
   }
 
+  handleClick(pageNumber, event) {
+    if (isSpecialClick(event)) {
+      // do not alter the default browser behavior
+      // if one special key is down
+      return;
+    }
+    event.preventDefault();
+    this.props.setCurrentPage(pageNumber);
+  }
+
   render() {
     let pager = new Paginator({
       currentPage: this.props.currentPage,

diff --git a/src/components/Pagination/PaginationLink.js b/src/components/Pagination/PaginationLink.js
@@ -1,8 +1,22 @@
 import React from 'react';
 
+import {isEqual} from 'lodash';
+
 class PaginationLink extends React.Component {
+  componentWillMount() {
+    this.handleClick = this.handleClick.bind(this);
+  }
+
+  shouldComponentUpdate(nextProps) {
+    return !isEqual(this.props, nextProps);
+  }
+
+  handleClick(e) {
+    this.props.handleClick(this.props.pageNumber, e);
+  }
+
   render() {
-    let {cssClasses, label, ariaLabel, handleClick, url} = this.props;
+    let {cssClasses, label, ariaLabel, url} = this.props;
 
     return (
       <li className={cssClasses.item}>
@@ -11,7 +25,7 @@ class PaginationLink extends React.Component {
           className={cssClasses.link}
           dangerouslySetInnerHTML={{__html: label}}
           href={url}
-          onClick={handleClick}
+          onClick={this.handleClick}
         ></a>
       </li>
     );
@@ -32,6 +46,7 @@ PaginationLink.propTypes = {
     React.PropTypes.string,
     React.PropTypes.number
   ]).isRequired,
+  pageNumber: React.PropTypes.number,
   url: React.PropTypes.string
 };
 

diff --git a/src/components/Pagination/__tests__/Pagination-test.js b/src/components/Pagination/__tests__/Pagination-test.js
@@ -62,6 +62,7 @@ describe('Pagination', () => {
     let createURL = sinon.stub().returns('/page');
     let out = new Pagination({cssClasses: {}}).pageLink({
       label: 'test',
+      pageNumber: 8,
       createURL
     });
 
@@ -70,8 +71,9 @@ describe('Pagination', () => {
         ariaLabel={undefined}
         cssClasses={{item: '', link: ''}}
         handleClick={() => {}}
-        key="test"
+        key="test8"
         label="test"
+        pageNumber={8}
         url="/page"
       />);
     expect(createURL.calledOnce).toBe(true, 'createURL should be called once');
@@ -82,6 +84,7 @@ describe('Pagination', () => {
     let out = new Pagination({cssClasses: {}}).pageLink({
       label: 'test',
       isDisabled: true,
+      pageNumber: 8,
       createURL
     });
 
@@ -90,8 +93,9 @@ describe('Pagination', () => {
         ariaLabel={undefined}
         cssClasses={{item: '', link: ''}}
         handleClick={() => {}}
-        key="test"
+        key="test8"
         label="test"
+        pageNumber={8}
         url="#"
       />);
     expect(createURL.called).toBe(false, 'createURL should not be called');

diff --git a/src/components/PoweredBy/PoweredBy.js b/src/components/PoweredBy/PoweredBy.js
@@ -1,6 +1,10 @@
 import React from 'react';
 
 class PoweredBy extends React.Component {
+  shouldComponentUpdate() {
+    return false;
+  }
+
   render() {
     return (
       <div className={this.props.cssClasses.root}>

diff --git a/src/components/PriceRanges/PriceRanges.js b/src/components/PriceRanges/PriceRanges.js
@@ -3,8 +3,18 @@ import React from 'react';
 import Template from '../Template.js';
 import PriceRangesForm from './PriceRangesForm.js';
 import cx from 'classnames';
+import {isEqual} from 'lodash';
 
 class PriceRanges extends React.Component {
+  componentWillMount() {
+    this.form = this.getForm();
+    this.refine = this.refine.bind(this);
+  }
+
+  shouldComponentUpdate(nextProps) {
+    return !isEqual(this.props.facetValues, nextProps.facetValues);
+  }
+
   getForm() {
     let labels = {
       currency: this.props.currency,
@@ -15,24 +25,16 @@ class PriceRanges extends React.Component {
       <PriceRangesForm
         cssClasses={this.props.cssClasses}
         labels={labels}
-        refine={this.refine.bind(this)}
+        refine={this.refine}
       />
     );
   }
 
-  getURLFromFacetValue(facetValue) {
-    if (!this.props.createURL) {
-      return '#';
-    }
-    return this.props.createURL(facetValue.from, facetValue.to, facetValue.isRefined);
-  }
-
   getItemFromFacetValue(facetValue) {
     let cssClassItem = cx(
       this.props.cssClasses.item,
       {[this.props.cssClasses.active]: facetValue.isRefined}
     );
-    let url = this.getURLFromFacetValue(facetValue);
     let key = facetValue.from + '_' + facetValue.to;
     let handleClick = this.refine.bind(this, facetValue.from, facetValue.to);
     let data = {
@@ -43,7 +45,7 @@ class PriceRanges extends React.Component {
       <div className={cssClassItem} key={key}>
         <a
           className={this.props.cssClasses.link}
-          href={url}
+          href={facetValue.url}
           onClick={handleClick}
         >
           <Template data={data} templateKey="item" {...this.props.templateProps} />
@@ -62,22 +64,20 @@ class PriceRanges extends React.Component {
   }
 
   render() {
-    let form = this.getForm();
     return (
       <div>
         <div className={this.props.cssClasses.list}>
           {this.props.facetValues.map(facetValue => {
             return this.getItemFromFacetValue(facetValue);
           })}
         </div>
-        {form}
+        {this.form}
       </div>
     );
   }
 }
 
 PriceRanges.propTypes = {
-  createURL: React.PropTypes.func.isRequired,
   cssClasses: React.PropTypes.shape({
     active: React.PropTypes.string,
     button: React.PropTypes.string,

diff --git a/src/components/PriceRanges/PriceRangesForm.js b/src/components/PriceRanges/PriceRangesForm.js
@@ -1,6 +1,14 @@
 import React from 'react';
 
 class PriceRangesForm extends React.Component {
+  componentWillMount() {
+    this.handleSubmit = this.handleSubmit.bind(this);
+  }
+
+  shouldComponentUpdate() {
+    return false;
+  }
+
   getInput(type) {
     return (
       <label className={this.props.cssClasses.label}>
@@ -19,7 +27,7 @@ class PriceRangesForm extends React.Component {
   render() {
     let fromInput = this.getInput('from');
     let toInput = this.getInput('to');
-    let onSubmit = this.handleSubmit.bind(this);
+    let onSubmit = this.handleSubmit;
     return (
       <form className={this.props.cssClasses.form} onSubmit={onSubmit} ref="form">
         {fromInput}

diff --git a/src/components/PriceRanges/__tests__/PriceRanges-test.js b/src/components/PriceRanges/__tests__/PriceRanges-test.js
@@ -58,46 +58,11 @@ describe('PriceRanges', () => {
       stubMethod('render');
     });
 
-    context('getURLFromFacetValue', () => {
-      it('should be a # if no createURL method passed', () => {
-        // Given
-        let component = getComponentWithMockRendering({
-          createURL: null
-        });
-
-        // When
-        let url = component.getURLFromFacetValue();
-
-        // Then
-        expect(url).toEqual('#');
-      });
-      it('should call the createURL method passed with the facetValue', () => {
-        // Given
-        let mockCreateURL = sinon.spy();
-        let component = getComponentWithMockRendering({
-          createURL: mockCreateURL
-        });
-        let facetValue = {
-          from: 6,
-          to: 8,
-          isRefined: true
-        };
-
-        // When
-        component.getURLFromFacetValue(facetValue);
-
-        // Then
-        expect(mockCreateURL.called).toBe(true);
-        expect(mockCreateURL.calledWith(6, 8, true)).toBe(true);
-      });
-    });
-
     context('getItemFromFacetValue', () => {
       let props;
       let facetValue;
 
       beforeEach(() => {
-        stubMethod('getURLFromFacetValue', 'url');
         props = {
           cssClasses: {
             item: 'item',
@@ -109,7 +74,8 @@ describe('PriceRanges', () => {
         facetValue = {
           from: 1,
           to: 10,
-          isRefined: false
+          isRefined: false,
+          url: 'url'
         };
       });