Skip to content

refacto: [CalendarHeader, ExpandableCalendar] replace defaultProps with ES6 default parameters #2480

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

david-mi
Copy link

@david-mi david-mi commented Jun 5, 2024

The issue

image

As we can read see in this article : React 19 RC Upgrade Guide#removed-deprecated-react-apis
defaultProps are being deprecated and won't be supported anymore with functional components in React 19

Suggested Fix

Simply replace concerned functional components (ExpandableCalendar and CalendarHeader) default props from with ES6 default parameters (some default parameters were already applied this way but defaultProps usage was remaining

@david-mi david-mi changed the title refacto(CalendarHeader, ExpandableCalendar): replace defaultProps with ES6 default parameters refacto [CalendarHeader, ExpandableCalendar]: replace defaultProps with ES6 default parameters Jun 5, 2024
@david-mi david-mi changed the title refacto [CalendarHeader, ExpandableCalendar]: replace defaultProps with ES6 default parameters refacto: [CalendarHeader, ExpandableCalendar] replace defaultProps with ES6 default parameters Jun 5, 2024
@maukoese
Copy link

Would be nice to merge this to remove the console error; and to be up to date.

@loganbnielsen
Copy link

can we get this reviewed?

@@ -621,15 +621,4 @@ const ExpandableCalendar = (props: ExpandableCalendarProps) => {
export default ExpandableCalendar;

ExpandableCalendar.displayName = 'ExpandableCalendar';
ExpandableCalendar.defaultProps = {

Choose a reason for hiding this comment

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

should we fill in the rest of these as well in src/calendar/header/index.tsx? I think firstDay is the only one that's enumerated above so we maybe replace it withfirstDay = 0.

Are the other props not used or why are they here?

Copy link

@loganbnielsen loganbnielsen Dec 30, 2024

Choose a reason for hiding this comment

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

Oh looks like the other ones are ExpandableCalendar props. I think we're also missing arrowsHitSlop in CalendarHeader?

Copy link

@Dallas62 Dallas62 left a comment

Choose a reason for hiding this comment

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

LGTM, all parameters are set correctly as far as I see.

@Dallas62
Copy link

Ping @nitzanyiz It includes some fix for new RN versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants