fix: #4558 Rename calendarIconClassname props to calendarIconClassName props

[yuki0410-dev]

Description

Linked issue: close #4558

Problem
See issue #4558

Changes
fix props : calendarIconClassname => calendarIconClassName

Screenshots

To reviewers

Contribution checklist

• I have followed the contributing guidelines.
• I have added sufficient test coverage for my changes.
• I have formatted my code with Prettier and checked for linting issues with ESLint for code readability.

[pullrequest]

✅ This pull request was sent to the PullRequest network.

---

@yuki0410-dev you can click here to see the review status or cancel the code review job.

[pullrequest]

PullRequest Breakdown

Reviewable lines of change

+ 78
- 5

69% JavaScript (tests)
29% JavaScript
2% Markdown

Type of change

Fix - These changes are likely to be fixing a bug or issue.

[martijnrusschen]

This may be a breaking change, what's the benefit of doing this?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.35%. Comparing base (8081b73) to head (b39399b).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4643   +/-   ##
=======================================
  Coverage   97.34%   97.35%           
=======================================
  Files          23       23           
  Lines        2603     2606    +3     
  Branches     1109     1124   +15     
=======================================
+ Hits         2534     2537    +3     
  Misses         69       69           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[pullrequest]

The change itself is good but this would break existing apps using react-datepicker that are using the old prop name. So this would likely need to either be versioned correctly or perhaps add a deprecation period for the old calendarIconClassname until users have migrated to calendarIconClassName

Ex: A quick look through github search shows some example apps that would break if they update their dependency:

https://github.com/search?q=%22react-datepicker%22+%22calendarIconClassname%22&type=code

Curtis Larson

---

Reviewed with ❤️ by PullRequest

[yuki0410-dev]

@martijnrusschen
According to the reasons given in the issue.

some class name properties is named xyzClassName, while others are called xyzClassname, this creates a bit of confusion when trying to override styling.

[pullrequest]

This prop rename and test addition look good to me. Nice work!

Joey G

---

Reviewed with ❤️ by PullRequest

[yuki0410-dev]

I tried to make the old props apply if you are not using the new props.
I also tried to output console.warn if you are using the old properties, what do you think?

[pullrequest]

I tried to make the old props apply if you are not using the new props.
I also tried to output console.warn if you are using the old properties, what do you think?

I'm not certain what the project's guidelines are around console statements since they are usually removed for production code. I think some sort of notice of deprecation in the README would be nice or even removing the calendarIconClassname in the README and let it silently work for existing projects but ensure future projects only use calendarIconClassName. These are likely decisions to be made by the project owner though since there are many different ways to accomplish deprecating fields.

Curtis Larson

[yuki0410-dev]

Thanks for the review.

I'm not certain what the project's guidelines are around console statements since they are usually removed for production code.

Agreed. However, since console.warn is often not deleted in development builds, I think it makes sense to leave the message in console.warn.

I think some sort of notice of deprecation in the README would be nice or even removing the calendarIconClassname in the README and let it silently work for existing projects but ensure future projects only use calendarIconClassName.

I agree, I have marked it as deprecated in datepicker.md, but not in index.md, as it will probably be overwritten by auto-generation, so no details are given.

These are likely decisions to be made by the project owner though since there are many different ways to accomplish deprecating fields.

Agreed.

@martijnrusschen
What do you think about this PR response?

[martijnrusschen]

There are currently 3 PRs in the list that contain breaking changes. Happy to make a major version upgrade if we can get all 3 merged.

[pullrequest]

PullRequest reviewed the updates made to #4643 up until the latest commit (366dfc4). No further issues were found.

Reviewed by:

Curtis Larson

[pullrequest]

PullRequest reviewed the updates made to #4643 up until the latest commit (631d7dc). No further issues were found.

Reviewed by:

Curtis Larson

[pullrequest]

Due to inactivity, PullRequest has cancelled this review job. You can reactivate the code review job from the PullRequest dashboard.

[mirus-ua]

Hello

This one will be major, so you can update your PR to include it in the release

[martijnrusschen]

@yuki0410-dev Can you merge the latest main branch to make sure this PR is still compatible?

[yuki0410-dev]

@martijnrusschen
I have imported the latest master and the CI results seem to be fine.