Skip to content

Commit

Permalink
Optimize isDayVisible
Browse files Browse the repository at this point in the history
Cloning is a little expensive, so we want to do it as little as possible.
Here we clone the month once and keep mutating that moment object.

In my profiling this change by itself appears to reduce the amount of
time spent in this function when updating DayPickeRangeController from
~16ms to ~11ms, with almost all of the time being spent in Moment clone.
  • Loading branch information
lencioni committed May 30, 2019
1 parent 4073d24 commit 0c35128
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 4 deletions.
20 changes: 16 additions & 4 deletions src/utils/isDayVisible.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,23 @@ import isAfterDay from './isAfterDay';

export default function isDayVisible(day, month, numberOfMonths, enableOutsideDays) {
if (!moment.isMoment(day)) return false;
let firstDayOfFirstMonth = month.clone().startOf('month');
if (enableOutsideDays) firstDayOfFirstMonth = firstDayOfFirstMonth.startOf('week');

// Cloning is a little expensive, so we want to do it as little as possible.
// Here we clone the month once and keep mutating that moment object.
const mutableMonth = month.clone();

const firstDayOfFirstMonth = enableOutsideDays
? mutableMonth.startOf('month').startOf('week')
: mutableMonth.startOf('month');

if (isBeforeDay(day, firstDayOfFirstMonth)) return false;

let lastDayOfLastMonth = month.clone().add(numberOfMonths - 1, 'months').endOf('month');
if (enableOutsideDays) lastDayOfLastMonth = lastDayOfLastMonth.endOf('week');
const lastDayOfLastMonth = enableOutsideDays
// We need to call endOf('week') when enableOutsideDays is true, because we
// are reusing the moment object, and our earlier mutation may have moved it
// to a previous month. This should snap us back to a good starting place.
? mutableMonth.endOf('week').add(numberOfMonths - 1, 'months').endOf('month').endOf('week')
: mutableMonth.add(numberOfMonths - 1, 'months').endOf('month');

return !isAfterDay(day, lastDayOfLastMonth);
}
28 changes: 28 additions & 0 deletions test/utils/isDayVisible_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,32 @@ describe('#isDayVisible', () => {
const currentMonth = moment().add(2, 'months');
expect(isDayVisible(test, currentMonth, 2)).to.equal(false);
});

describe('enableOutsideDays', () => {
it('returns true if arg is in partial week before visible months', () => {
const test = moment('2019-04-30');
const currentMonth = moment('2019-05-01');
expect(isDayVisible(test, currentMonth, 1, false)).to.equal(false);
expect(isDayVisible(test, currentMonth, 1, true)).to.equal(true);
});

it('returns true if arg is in partial week after visible months', () => {
const test = moment('2019-06-01');
const currentMonth = moment('2019-05-01');
expect(isDayVisible(test, currentMonth, 1, false)).to.equal(false);
expect(isDayVisible(test, currentMonth, 1, true)).to.equal(true);
});

it('returns false if arg is before partial week before visible months', () => {
const test = moment('2019-04-27');
const currentMonth = moment('2019-05-01');
expect(isDayVisible(test, currentMonth, 1, true)).to.equal(false);
});

it('returns false if arg is after partial week after visible months', () => {
const test = moment('2019-06-03');
const currentMonth = moment('2019-05-01');
expect(isDayVisible(test, currentMonth, 1, true)).to.equal(false);
});
});
});

0 comments on commit 0c35128

Please sign in to comment.