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

Fix - Removed defaultProps from Function Components for React 18.3. #4498

Merged
merged 1 commit into from Feb 2, 2024

Conversation

shyk001
Copy link
Contributor

@shyk001 shyk001 commented Feb 2, 2024

As of react 18.3, a warning will be displayed when using defaultProps inside a function component. Therefore, I have removed the defaultProps used in the function component.

Fix #4477

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

✅ This pull request was sent to the PullRequest network.


@shyk001 you can click here to see the review status or cancel the code review job.

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

PullRequest Breakdown

Reviewable lines of change

+ 12
- 11

100% JavaScript

Type of change

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

Copy link

codecov bot commented Feb 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7344120) 95.39% compared to head (8839334) 95.39%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4498   +/-   ##
=======================================
  Coverage   95.39%   95.39%           
=======================================
  Files          29       29           
  Lines        2518     2521    +3     
  Branches     1018     1021    +3     
=======================================
+ Hits         2402     2405    +3     
  Misses        116      116           

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

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

this change looks good to me and matching the description regarding addressing the defaultProps usage

Image of Xiaoyong W Xiaoyong W


Reviewed with ❤️ by PullRequest

@martijnrusschen martijnrusschen merged commit 60609a7 into Hacker0x01:main Feb 2, 2024
6 checks passed
@reinrl
Copy link

reinrl commented Feb 5, 2024

@shyk001 Is the extra typeof check in the line hidePopper: typeof props.hidePopper === "boolean" ? props.hidePopper : true because otherwise you couldn't ever set hidePopper to false? Why not just use nullish coalescing (e.g., hidePopper: props.hidePopper ?? true - which you could use for all three default prop comparisons), or just create a default props object within the component body and then layer the incoming props in on top of that?

const defaultProps = {
  popperModifiers: [],
  popperProps: {},
  hidePopper: true,
};
const alt_props = {
  ...defaultProps,
  ...props,
};

@shyk001
Copy link
Contributor Author

shyk001 commented Feb 6, 2024

@reinrl
I think it would have been good code to use nullish coalescing like you said, I missed it. Thanks for the advice.

For the second method, I don't think it will work as you intended because the props will overwrite the default_props.

const props = {
    a: "1qe",
    b: "asdf",
    c: undefined,
    d: {a: "gkgk"},
    e: undefined
}

const default_props = {
    c: [],
    d: {},
    e: true,
}

const alt_props = {
    ...default_props,
    ...props
}

# alt_props console log result
# { c: undefined, d: { a: 'gkgk' }, e: undefined, a: '1qe', b: 'asdf' }

@reinrl
Copy link

reinrl commented Feb 6, 2024

@shyk001 good catch - explicitly passing in undefined for any of the props was an edge case that I hadn't considered. Just took me longer than I care to admit to figure out the "why" behind the Boolean type check in your change...and then once I figured it out, I started thinking through other alternatives I have used elsewhere (and your point is a good one that is going to make me revisit some past code elsewhere).

fkoulen added a commit to ASVGay/the-rhapsodies that referenced this pull request Feb 17, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [react-datepicker](https://togithub.com/Hacker0x01/react-datepicker) |
[`^4.14.0` ->
`^6.0.0`](https://renovatebot.com/diffs/npm/react-datepicker/4.25.0/6.1.0)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/react-datepicker/6.1.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/react-datepicker/6.1.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/react-datepicker/4.25.0/6.1.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/react-datepicker/4.25.0/6.1.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
|
[@types/react-datepicker](https://togithub.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/react-datepicker)
([source](https://togithub.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/react-datepicker))
| [`^4.11.2` ->
`^6.0.0`](https://renovatebot.com/diffs/npm/@types%2freact-datepicker/4.19.6/6.0.1)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/@types%2freact-datepicker/6.0.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@types%2freact-datepicker/6.0.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@types%2freact-datepicker/4.19.6/6.0.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@types%2freact-datepicker/4.19.6/6.0.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>Hacker0x01/react-datepicker (react-datepicker)</summary>

###
[`v6.1.0`](https://togithub.com/Hacker0x01/react-datepicker/releases/tag/v6.1.0):
6.1.0

[Compare
Source](https://togithub.com/Hacker0x01/react-datepicker/compare/v6.0.0...v6.1.0)

#### What's Changed

- Fix - Removed defaultProps from Function Components for React 18.3. by
[@&#8203;shyk001](https://togithub.com/shyk001) in
[Hacker0x01/react-datepicker#4498

#### New Contributors

- [@&#8203;shyk001](https://togithub.com/shyk001) made their first
contribution in
[Hacker0x01/react-datepicker#4498

**Full Changelog**:
Hacker0x01/react-datepicker@v6.0.0...v6.1.0

###
[`v6.0.0`](https://togithub.com/Hacker0x01/react-datepicker/releases/tag/v6.0.0):
6.0.0

[Compare
Source](https://togithub.com/Hacker0x01/react-datepicker/compare/v5.1.0...v6.0.0)

#### What's Changed

- Upgrade date-fns to v3 by
[@&#8203;ethanve](https://togithub.com/ethanve) in
[Hacker0x01/react-datepicker#4481
- Switch workflows to Node 20 by
[@&#8203;martijnrusschen](https://togithub.com/martijnrusschen) in
[Hacker0x01/react-datepicker#4490

#### New Contributors

- [@&#8203;ethanve](https://togithub.com/ethanve) made their first
contribution in
[Hacker0x01/react-datepicker#4481

**Full Changelog**:
Hacker0x01/react-datepicker@v5.1.0...v6.0.0

###
[`v5.1.0`](https://togithub.com/Hacker0x01/react-datepicker/releases/tag/v5.1.0):
5.1.0

[Compare
Source](https://togithub.com/Hacker0x01/react-datepicker/compare/v5.0.0...v5.1.0)

#### What's Changed

- Fix options passed to date-fns/parse by
[@&#8203;emilecantin](https://togithub.com/emilecantin) in
[Hacker0x01/react-datepicker#4474

**Full Changelog**:
Hacker0x01/react-datepicker@v5.0.0...v5.1.0

###
[`v5.0.0`](https://togithub.com/Hacker0x01/react-datepicker/releases/tag/v5.0.0):
5.0.0

[Compare
Source](https://togithub.com/Hacker0x01/react-datepicker/compare/v4.25.0...v5.0.0)

#### Breaking changes

- Migrate from Popper.js to Floating-UI by
[@&#8203;G07cha](https://togithub.com/G07cha) in
[Hacker0x01/react-datepicker#4393

#### What's Changed

- 🐛 FIX: readability-isMonthinRange by
[@&#8203;mary139](https://togithub.com/mary139) in
[Hacker0x01/react-datepicker#4421
- Fix
[#&#8203;4431](https://togithub.com/Hacker0x01/react-datepicker/issues/4431):
Update the excludedDate to match the year to check of the isYearDisabled
by [@&#8203;balajis-qb](https://togithub.com/balajis-qb) in
[Hacker0x01/react-datepicker#4432
- Fix
[#&#8203;4420](https://togithub.com/Hacker0x01/react-datepicker/issues/4420):
Update home key and end key navigation in Calendar component by
[@&#8203;balajis-qb](https://togithub.com/balajis-qb) in
[Hacker0x01/react-datepicker#4430
- Document
[#&#8203;4420](https://togithub.com/Hacker0x01/react-datepicker/issues/4420):
📝 Update the behavior of Home Key and End Key in the README file by
[@&#8203;balajis-qb](https://togithub.com/balajis-qb) in
[Hacker0x01/react-datepicker#4438
- Fix
[#&#8203;4076](https://togithub.com/Hacker0x01/react-datepicker/issues/4076):
Trigger onCalendarClose event and onChange even when the same date is
selected as the start and the end date in a date range by
[@&#8203;balajis-qb](https://togithub.com/balajis-qb) in
[Hacker0x01/react-datepicker#4394
- Excluded dates message by
[@&#8203;dvelazquez1282](https://togithub.com/dvelazquez1282) in
[Hacker0x01/react-datepicker#4437
- Fix
[#&#8203;4456](https://togithub.com/Hacker0x01/react-datepicker/issues/4456):
Add shift+pageUp key and shift+pageDown key navigation in Calendar
component by [@&#8203;balajis-qb](https://togithub.com/balajis-qb) in
[Hacker0x01/react-datepicker#4457
- Fix options passed to date-fns/format by
[@&#8203;emilecantin](https://togithub.com/emilecantin) in
[Hacker0x01/react-datepicker#4469

#### New Contributors

- [@&#8203;mary139](https://togithub.com/mary139) made their first
contribution in
[Hacker0x01/react-datepicker#4421
- [@&#8203;G07cha](https://togithub.com/G07cha) made their first
contribution in
[Hacker0x01/react-datepicker#4393
- [@&#8203;dvelazquez1282](https://togithub.com/dvelazquez1282) made
their first contribution in
[Hacker0x01/react-datepicker#4437
- [@&#8203;emilecantin](https://togithub.com/emilecantin) made their
first contribution in
[Hacker0x01/react-datepicker#4469

**Full Changelog**:
Hacker0x01/react-datepicker@v4.25.0...v5.0.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these
updates again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/ASVGay/the-rhapsodies).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xNzMuMCIsInVwZGF0ZWRJblZlciI6IjM3LjE3My4wIiwidGFyZ2V0QnJhbmNoIjoiZGV2In0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants