Optimize toISODateString and toISOMonthString #1656
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lencioni
added
the
semver-patch: fixes/refactors/etc
ljharb
reviewed
May 30, 2019
|
@ljharb I added a test for toISODateString but not toISOMonthString because the existing tests there were all written in this way. |
ljharb
approved these changes
May 30, 2019
test/utils/toISOMonthString_spec.js
Outdated
| @@ -1,9 +1,11 @@ | |||
| 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
