-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[pickers] Do not require date-fns
in @mui/x-date-pickers-pro
#5941
[pickers] Do not require date-fns
in @mui/x-date-pickers-pro
#5941
Conversation
These are the results for the performance tests:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's an unexpected consequence of the experiment 🤯
I confirm the regression is fixed by this PR
issue reproduction: https://codesandbox.io/s/admiring-bird-djug44?file=/index.tsx
issue reproduction with the fix: https://codesandbox.io/s/bold-germain-jt0dsl?file=/demo.tsx:133-170
// We don't add those exports to the regular `@mui/x-date-pickers/internals`, | ||
// Because they rely on date-fns, which is not used by all applications. | ||
export { | ||
useField, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say that we should still look into removing the direct dependency of date-fns
from useField.utls
going forward 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I think we all agree on that
I requires new methods in the adapter so it was not feasable for a simple POC
But the components can't be stable with hardcoded date-fns stuff
…#5941) [pickers] Do not require date-fns in @mui/x-date-pickers-pro
Fixes #5916