Skip to content
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] Remove the defaultCalendarMonth prop #10987

Merged
merged 17 commits into from
Nov 22, 2023

Conversation

flaviendelangle
Copy link
Member

@flaviendelangle flaviendelangle commented Nov 10, 2023

Closes #10986

I have two questions here:

  1. I forgot to deprecate this prop when releasing referenceDate. Do you think it's a good enough reason to postpone its removal to v8? The migration to referenceDate is very straightforward and the prop is not widely used so I'd say it's fine but if you want we can keep it another year.

  2. Should we codemod it and add it to the preset-safe? This is not a 1-on-1 behavior since the referenceDate also applies to other views, so it could lead to unwated behavior. Should we create the codemod but let people run it if they want?

Changelog

Breaking changes

  • The deprecated defaultCalendarMonth prop has been removed in favor of the more flexible referenceDate prop.

    - <DateCalendar defaultCalendarMonth={dayjs('2022-04-01')};
    + <DateCalendar referenceDate{dayjs('2022-04-01')} />

@flaviendelangle flaviendelangle self-assigned this Nov 10, 2023
@flaviendelangle flaviendelangle added breaking change component: pickers This is the name of the generic UI component, not the React module! v7.x labels Nov 10, 2023
@flaviendelangle flaviendelangle changed the title [pickers] Remove the defaultCalendarMonth [pickers] Remove the defaultCalendarMonth prop Nov 10, 2023
@mui-bot
Copy link

mui-bot commented Nov 10, 2023

@alexfauquette
Copy link
Member

I forgot to deprecate this prop when releasing referenceDate. Do you think it's a good enough reason to postpone its removal to v8? The migration to referenceDate is very straightforward and the prop is not widely used so I'd say it's fine but if you want we can keep it another year.

For me you can remove defaultCalendarMonth. The pain of replacing it with referenceDate is a small breaking change.

Should we codemod it and add it to the preset-safe? This is not a 1-on-1 behavior since the referenceDate also applies to other views, so it could lead to unwated behavior. Should we create the codemod but let people run it if they want?

Not sure it's worth creating a codemod if not in preset-safe. The reason is that I'm reading the breaking change. If I need to think, it's better to have the part of the codebase using it under my eyes. So I would prefer to do a search in the codebase to know where I'm using defaultCalendarMonth to think what should I change.

Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 10, 2023
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 10, 2023
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 14, 2023
@github-actions github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Nov 14, 2023
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 16, 2023
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 20, 2023
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 20, 2023
Copy link
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1. I forgot to deprecate this prop when releasing referenceDate. Do you think it's a good enough reason to postpone its removal to v8? The migration to referenceDate is very straightforward and the prop is not widely used so I'd say it's fine but if you want we can keep it another year.

WDYT about deprecated it after the fact - add the deprecation in v5 just now.
Imagine users using v6 maybe for 6 or 12 months after the release of new stable. They'd see the deprecation warning and potentially migrate to the referenceDate before upgrading to v7. 🤔

2. Should we codemod it and add it to the preset-safe? This is not a 1-on-1 behavior since the referenceDate also applies to other views, so it could lead to unwated behavior. Should we create the codemod but let people run it if they want?

I'd be in favor of creating the codemod if it's not that difficult. 🤔
As for adding it into the preset-safe. Well, that's debatable. We could omit it just for the sake of complete correctness, although, I'd imagine, that almost no one would mind the slight change of behavior. 🙈 🤷

@flaviendelangle
Copy link
Member Author

WDYT about deprecated it after the fact - add the deprecation in v5 just now.

To deprecate it on the next v6 patch?
It's a good idea!

@LukasTy
Copy link
Member

LukasTy commented Nov 21, 2023

WDYT about deprecated it after the fact - add the deprecation in v5 just now.

To deprecate it on the next v6 patch? It's a good idea!

Yes, that's exactly my idea. 🤔
IMHO, it's way better than just removing the prop without deprecation and probably better than keeping it deprecated on v7. 🙈

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 22, 2023
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@flaviendelangle
Copy link
Member Author

@LukasTy #11138

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 22, 2023
Copy link
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👏

@flaviendelangle flaviendelangle merged commit 037d1ef into mui:next Nov 22, 2023
17 checks passed
@flaviendelangle flaviendelangle deleted the remove-defaultCalendarMonth branch November 22, 2023 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: pickers This is the name of the generic UI component, not the React module! v7.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[pickers] v7: Remove the defaultCalendarMonth prop
4 participants