Repository: gpbl/react-day-picker
Pull request #452 (2017-07-07 05:10:20)

Bugfix: Better Day.shouldComponentUpdate implementation and exporting…
**Bugs:**
Fixes https://github.com/gpbl/react-day-picker/issues/446

The `Day.shouldComponentUpdate` method was not allowing days to update when anything other than modifiers changed.  This can lead to buggy behavior if many public props of DayPicker change.

Also, the NavbarPropTypes and WeekdayPropTypes were being exported incorrectly starting here: https://github.com/gpbl/react-day-picker/commit/0b5a46f91faf5179c945e97a3fd273982143f237

**Changes:**
- Export propTypes directly off of components inside of `DayPicker.js` where appropriate.
- For most props in `Day`, do a shallow compare to determine the return value of `shouldComponentUpdate`.  However, `modifiers` and `modifiersStyles` need their own shallow compare as well, so make an exception for them.

cc: @gpbl 

codecov[bot]: # [Codecov](https://codecov.io/gh/gpbl/react-day-picker/pull/452?src=pr&el=h1) Report
> Merging [#452](https://codecov.io/gh/gpbl/react-day-picker/pull/452?src=pr&el=desc) into [master](https://codecov.io/gh/gpbl/react-day-picker/commit/dd54b02c95d466a082b16482f85e4737fb1723a7?src=pr&el=desc) will **decrease** coverage by `1.71%`.
> The diff coverage is `47.05%`.

[![Impacted file tree graph](https://codecov.io/gh/gpbl/react-day-picker/pull/452/graphs/tree.svg?width=650&token=1S9igPKpZd&height=150&src=pr)](https://codecov.io/gh/gpbl/react-day-picker/pull/452?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #452      +/-   ##
==========================================
- Coverage     100%   98.28%   -1.72%     
==========================================
  Files          15       15              
  Lines         512      526      +14     
  Branches      104      108       +4     
==========================================
+ Hits          512      517       +5     
- Misses          0        7       +7     
- Partials        0        2       +2
```


| [Impacted Files](https://codecov.io/gh/gpbl/react-day-picker/pull/452?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/Day.js](https://codecov.io/gh/gpbl/react-day-picker/pull/452?src=pr&el=tree#diff-c3JjL0RheS5qcw==) | `74.28% <47.05%> (-25.72%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/gpbl/react-day-picker/pull/452?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/gpbl/react-day-picker/pull/452?src=pr&el=footer). Last update [dd54b02...77d7063](https://codecov.io/gh/gpbl/react-day-picker/pull/452?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/DayPicker.js b/DayPicker.js
index 084214e93..a27fff5c6 100644
--- a/DayPicker.js
+++ b/DayPicker.js
@@ -5,14 +5,14 @@ var DayPicker = require('./lib/src/DayPicker');
 var DateUtils = require('./lib/src/DateUtils');
 var LocaleUtils = require('./lib/src/LocaleUtils');
 var ModifiersUtils = require('./lib/src/ModifiersUtils');
-var Weekday = require('./lib/src/Weekday');
-var Navbar = require('./lib/src/Navbar');
+var Weekday = require('./lib/src/Weekday').default;
+var Navbar = require('./lib/src/Navbar').default;
 var PropTypes = require('./lib/src/PropTypes');
 
 module.exports = DayPicker.default || DayPicker;
 module.exports.DateUtils = DateUtils.default || DateUtils;
 module.exports.LocaleUtils = LocaleUtils.default || LocaleUtils;
 module.exports.ModifiersUtils = ModifiersUtils.default || ModifiersUtils;
-module.exports.WeekdayPropTypes = Weekday.WeekdayPropTypes;
-module.exports.NavbarPropTypes = Navbar.NavbarPropTypes;
+module.exports.WeekdayPropTypes = Weekday.propTypes;
+module.exports.NavbarPropTypes = Navbar.propTypes;
 module.exports.PropTypes = PropTypes;
diff --git a/src/Day.js b/src/Day.js
index 4eea7467d..1004fb87c 100644
--- a/src/Day.js
+++ b/src/Day.js
@@ -3,6 +3,7 @@
 import React, { Component } from 'react';
 import assign from 'object-assign';
 import PropTypes from 'prop-types';
+import { isSameDay } from './DateUtils';
 
 import defaultClassNames from './classNames';
 
@@ -16,6 +17,9 @@ function handleEvent(handler, day, modifiers) {
   };
 }
 
+const hasOwnProp = (obj, prop) =>
+  Object.prototype.hasOwnProperty.call(obj, prop);
+
 export default class Day extends Component {
   static propTypes = {
     classNames: PropTypes.shape({
@@ -47,13 +51,40 @@ export default class Day extends Component {
 
   static defaultProps = {
     modifiers: {},
+    modifiersStyles: {},
     empty: false,
   };
 
   shouldComponentUpdate(nextProps) {
-    const modifiers = Object.keys(this.props.modifiers).sort();
-    const nextModifiers = Object.keys(nextProps.modifiers).sort();
-    return modifiers.join() !== nextModifiers.join();
+    const propNames = Object.keys(this.props);
+    const nextPropNames = Object.keys(nextProps);
+    if (propNames.length !== nextPropNames.length) {
+      return true;
+    }
+    return propNames.some(name => {
+      if (
+        name === 'modifiers' ||
+        name === 'modifiersStyles' ||
+        name === 'classNames'
+      ) {
+        const prop = this.props[name];
+        const nextProp = nextProps[name];
+        const modifiers = Object.keys(prop);
+        const nextModifiers = Object.keys(nextProp);
+        if (modifiers.length !== nextModifiers.length) {
+          return true;
+        }
+        return modifiers.some(
+          mod => !hasOwnProp(nextProp, mod) || prop[mod] !== nextProp[mod]
+        );
+      }
+      if (name === 'day') {
+        return !isSameDay(this.props[name], nextProps[name]);
+      }
+      return (
+        !hasOwnProp(nextProps, name) || this.props[name] !== nextProps[name]
+      );
+    });
   }
 
   render() {
diff --git a/src/DayPicker.js b/src/DayPicker.js
index af34209e2..2642b8724 100644
--- a/src/DayPicker.js
+++ b/src/DayPicker.js
@@ -400,7 +400,7 @@ export default class DayPicker extends Component {
 
   handleDayClick = (day, modifiers, e) => {
     e.persist();
-    if (modifiers[this.props.classNames.outside]) {
+    if (modifiers.outside) {
       this.handleOutsideDayClick(day);
     }
     if (this.props.onDayClick) {


Test patch:
diff --git a/test/daypicker/navigation.js b/test/daypicker/navigation.js
index 0a597cc0d..a0463d154 100644
--- a/test/daypicker/navigation.js
+++ b/test/daypicker/navigation.js
@@ -3,7 +3,6 @@ import { shallow, mount } from 'enzyme';
 
 import DayPicker from '../../src/DayPicker';
 import * as keys from '../../src/keys';
-import defaultClassNames from '../../src/classNames';
 
 describe('DayPicker’s navigation', () => {
   it('should not allow the previous month when the first month is the first allowed one', () => {
@@ -55,20 +54,6 @@ describe('DayPicker’s navigation', () => {
     expect(instance.state.currentMonth.getFullYear()).toBe(2015);
     expect(instance.state.currentMonth.getMonth()).toBe(8);
   });
-  it('should show the next month when clicking outside days with classNames set', () => {
-    const wrapper = mount(
-      <DayPicker
-        initialMonth={new Date(2015, 7)}
-        enableOutsideDays
-        classNames={{ ...defaultClassNames, outside: 'fakeOutside' }}
-        onDayClick={() => {}}
-      />
-    );
-    wrapper.find('.fakeOutside').last().simulate('click');
-    const instance = wrapper.instance();
-    expect(instance.state.currentMonth.getFullYear()).toBe(2015);
-    expect(instance.state.currentMonth.getMonth()).toBe(8);
-  });
   it('should show the previous month when clicking the previous button', () => {
     const wrapper = mount(<DayPicker initialMonth={new Date(2015, 7)} />);
     wrapper.find('.DayPicker-NavButton--prev').simulate('click');
@@ -89,20 +74,6 @@ describe('DayPicker’s navigation', () => {
     expect(instance.state.currentMonth.getFullYear()).toBe(2015);
     expect(instance.state.currentMonth.getMonth()).toBe(6);
   });
-  it('should show the previous month when clicking outside days with classNames set', () => {
-    const wrapper = mount(
-      <DayPicker
-        initialMonth={new Date(2015, 7)}
-        enableOutsideDays
-        classNames={{ ...defaultClassNames, outside: 'fakeOutside' }}
-        onDayClick={() => {}}
-      />
-    );
-    wrapper.find('.fakeOutside').first().simulate('click');
-    const instance = wrapper.instance();
-    expect(instance.state.currentMonth.getFullYear()).toBe(2015);
-    expect(instance.state.currentMonth.getMonth()).toBe(6);
-  });
   it('should not show the previous month when clicking outside days from the first month out of 2', () => {
     const wrapper = mount(
       <DayPicker
diff --git a/test/daypicker/rendering.js b/test/daypicker/rendering.js
index 0c9bd01f5..7043917fc 100644
--- a/test/daypicker/rendering.js
+++ b/test/daypicker/rendering.js
@@ -5,6 +5,7 @@ import { isElement } from 'react-dom/test-utils';
 import { shallow, mount, render } from 'enzyme';
 
 import DayPicker from '../../src/DayPicker';
+import Day from '../../src/Day';
 import classNames from '../../src/classNames';
 
 describe('DayPicker’s rendering', () => {
@@ -362,3 +363,129 @@ describe('DayPicker’s rendering', () => {
     expect(wrapper.find('.bar')).toHaveLength(1);
   });
 });
+
+describe('Day.shouldComponentUpdate', () => {
+  it('should return false if the modifiers object passes a shallow compare', () => {
+    const initial = {
+      a: true,
+      b: true,
+    };
+    const updated = {
+      b: true,
+      a: true,
+    };
+    const day = shallow(
+      <Day day={new Date()} classNames={{ day: 'day' }} modifiers={initial}>
+        2
+      </Day>
+    ).instance();
+    const newProps = Object.assign({}, day.props, {
+      modifiers: updated,
+    });
+    expect(day.shouldComponentUpdate(newProps)).toBeFalsy();
+  });
+
+  it('should return false if a new day date is passed that is in the same day', () => {
+    const day = shallow(
+      <Day day={new Date()} classNames={{ day: 'day' }}>
+        2
+      </Day>
+    ).instance();
+    const newProps = Object.assign({}, day.props, {
+      day: new Date(),
+    });
+    expect(day.shouldComponentUpdate(newProps)).toBeFalsy();
+  });
+
+  it('should return false if the modifiersStyles object passes a shallow compare', () => {
+    const initial = {
+      a: {},
+      b: {},
+    };
+    const updated = {
+      b: initial.b,
+      a: initial.a,
+    };
+    const day = shallow(
+      <Day
+        day={new Date()}
+        classNames={{ day: 'day' }}
+        modifiersStyles={initial}
+      >
+        2
+      </Day>
+    ).instance();
+    const newProps = Object.assign({}, day.props, {
+      modifiersStyles: updated,
+    });
+    expect(day.shouldComponentUpdate(newProps)).toBeFalsy();
+  });
+
+  it('should return false when empty does not change', () => {
+    const day = shallow(
+      <Day day={new Date()} classNames={{ day: 'day' }} empty>
+        2
+      </Day>
+    ).instance();
+    const newProps = Object.assign({}, day.props, {
+      empty: true,
+    });
+    expect(day.shouldComponentUpdate(newProps)).toBeFalsy();
+  });
+
+  it('should return true when modifiers change', () => {
+    const initial = {
+      a: true,
+      b: true,
+    };
+    const updated = {
+      c: true,
+      a: true,
+    };
+    const day = shallow(
+      <Day day={new Date()} classNames={{ day: 'day' }} modifiers={initial}>
+        2
+      </Day>
+    ).instance();
+    const newProps = Object.assign({}, day.props, {
+      modifiers: updated,
+    });
+    expect(day.shouldComponentUpdate(newProps)).toBeTruthy();
+  });
+
+  it('should return true when the day changes', () => {
+    const day = shallow(
+      <Day day={new Date()} classNames={{ day: 'day' }}>
+        2
+      </Day>
+    ).instance();
+    const newProps = Object.assign({}, day.props, {
+      day: new Date(2015, 1, 1),
+    });
+    expect(day.shouldComponentUpdate(newProps)).toBeTruthy();
+  });
+
+  it('should return true when empty changes', () => {
+    const day = shallow(
+      <Day day={new Date()} classNames={{ day: 'day' }} empty>
+        2
+      </Day>
+    ).instance();
+    const newProps = Object.assign({}, day.props, {
+      empty: false,
+    });
+    expect(day.shouldComponentUpdate(newProps)).toBeTruthy();
+  });
+
+  it('should return true when you add a prop', () => {
+    const day = shallow(
+      <Day day={new Date()} classNames={{ day: 'day' }} empty>
+        2
+      </Day>
+    ).instance();
+    const newProps = Object.assign({}, day.props, {
+      onKeyDown: () => {},
+    });
+    expect(day.shouldComponentUpdate(newProps)).toBeTruthy();
+  });
+});