-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[pickers] Avoid useless date creation in AdapterDayjs
#18429
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] Avoid useless date creation in AdapterDayjs
#18429
Conversation
Deploy preview: https://deploy-preview-18429--material-ui-x.netlify.app/ Bundle size reportTotal Size Change: ▼-586B(0.00%) - Total Gzip Change: ▼-160B(0.00%) Show details for 100 more bundles (22 more not shown)@mui/x-date-pickers-pro/AdapterDayjs parsed: ▼-293B(-3.47%) gzip: ▼-80B(-3.02%) |
AdapterDayjs
AdapterDayjs
1119c8a
to
b2a04b7
Compare
return this.adjustOffset( | ||
amount < 0 ? value.subtract(Math.abs(amount), 'year') : value.add(amount, 'year'), | ||
); | ||
return this.adjustOffset(value.add(amount, 'year')); |
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 noticed that value.subtract
just calls value.add
5a70ace
to
0935d0b
Compare
@@ -149,8 +143,6 @@ export class AdapterDayjs implements MuiPickersAdapter<string> { | |||
|
|||
public lib = 'dayjs'; | |||
|
|||
public dayjs: Constructor; |
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.
Strictly speaking, this is a breaking change.
Do you think it's worth keeping it (with withLocale
) even if we never use it?
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.
Since we do not document it and not use it internally as well I think we can remove it.
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.
LGTM ...
@@ -149,8 +143,6 @@ export class AdapterDayjs implements MuiPickersAdapter<string> { | |||
|
|||
public lib = 'dayjs'; | |||
|
|||
public dayjs: Constructor; |
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.
Since we do not document it and not use it internally as well I think we can remove it.
ac401d4
to
8e9c9ff
Compare
Part of #18410