-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proposal: parseISO from date-fns is pretty slow, we can replace parseISO from date-fns with Date() #29007
Comments
mountiny
added
Weekly
KSv2
Bug
Something is broken. Auto assigns a BugZero manager.
labels
Oct 6, 2023
Current assignee @muttmuure is eligible for the Bug assigner, not assigning anyone new. |
Bug0 Triage Checklist (Main S/O)
|
@waterim did you get to this by any chance? |
We got two issues for this 😂 closing in favour of #29271 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Coming from here
### Problem
date-fns is a great library but it’s underperforming in some scenarios For example, using parseISO function takes around 112 ms and it’s a lot of time for a utility function. The main problem we have at our hands is most of the work is already been done in moving from moment to date-fns . While date-fns is a good library and works well in other scenarios, which otherwise haven’t been reported by profiler but parseISO is such a utility function which is consuming noticeable milliseconds and reported by profiler.
Solution:
We have two solutions here:
Since we have spent a lot of efforts in migrating from moment to date-fns and certainly it’s better than moment but in some scenarios, some utility functions of date-fns are underperforming. Ideally, we can replace those underperforming parts with new Date and in future, if there are any other underperforming utility functions reported by Profiler, we can decide whether it’s fixable by using stock Date or now it is time to gradually adopt dayjs or any other efficient date library.
Conclusion
Lets use the Date() construct for now, we dont really have a clear problem which would be solved by introducing the
dayjs
right nowThe text was updated successfully, but these errors were encountered: