Optimize toISODateString and toISOMonthString#1656
Conversation
lencioni
added
the
semver-patch: fixes/refactors/etc
ljharb
left a comment
ljharb
left a comment
There was a problem hiding this comment.
Can we add tests comparing the output of this function to the moment format method directly, to ensure there’s no drift?
|
@ljharb I added a test for toISODateString but not toISOMonthString because the existing tests there were all written in this way. |
| import moment from 'moment'; | ||
| import { expect } from 'chai'; | ||
|
|
||
| import { ISO_FORMAT, ISO_MONTH_FORMAT } from '../../src/constants'; |
There was a problem hiding this comment.
why inline this, if the constant is still going to exist?
There was a problem hiding this comment.
It is only used in one test file now, and in no production code, so I don't think it makes sense to ship this code to clients.
There was a problem hiding this comment.
sure but this PR doesn’t delete the constant (which would be a breaking change). Can the inlining wait til then?
There was a problem hiding this comment.
Oops, I forgot to save that file. Yeah, I can keep it for now. I'll add a note to delete it as part of the next breaking change.
lencioni
force-pushed
the
iso-format
branch
2 times, most recently
from
2d7fd47 to
f888d7d
Compare
These functions get called repeatedly when computing modifiers, which happens any time components like DayPickerRangeController receives props. By manually building this string instead of relying on moment's format function, we can get better performance. In my profiling, this cuts the time spent in DayPickerRangeController#componentWillReceiveProps from ~50ms to ~40ms.
3 participants
These functions get called repeatedly when computing modifiers, which
happens any time components like DayPickerRangeController receives
props.
By manually building this string instead of relying on moment's format
function, we can get better performance. In my profiling, this cuts the
time spent in DayPickerRangeController#componentWillReceiveProps from
~50ms to ~40ms.
Before:
After:
