Remove defaultProps usages for functional components #7224

undeletable · 2024-05-09T19:23:35Z

What does this PR do?

Starting from React 19, defaultProps for functional components is deprecated. Starting from React 18.3, the corresponding error is thrown for a functional component with defaultProps defined.

Changes in this PR:

replace defaultProps for functional components with default function parameters values;
use attrs method of styled-components to pass theme to styled components which might be outside of theme context (required per remove defaultProps from Grommet #6741 (comment));
add custom hook to obtain theme from context with fallback to default value;
configure eslint rule to not require defaultProps for functional components;
update snapshots.

Where should the reviewer start?

No specific place to start.

What testing has been done on this PR?

manual testing on storybook and real project using grommet;
tests run result:

Test Suites: 97 passed, 97 total
Tests:       1548 passed, 1548 total
Snapshots:   1777 passed, 1777 total

How should this be manually tested?

Use React 18.3+ in an app. Check that styles are applied correctly and no errors about defaultProps are thrown into console.

Do Jest tests follow these best practices?

screen is used for querying.
The correct query is used. (Refer to this list of queries)
asFragment() is used for snapshot testing.

Any background context you want to provide?

React 19 upgrade guide

What are the relevant issues?

Closes #6741

Screenshots (if appropriate)

N/A.

Do the grommet docs need to be updated?

Just the internal ones (if any).

Should this PR be mentioned in the release notes?

Yes.

Is this change backwards compatible or is it a breaking change?

Changes are backwards compatible.

src/js/components/Menu/__tests__/Menu-test.js

Signed-off-by: Max Shepel <max@undeletable.name>

…parameter value Signed-off-by: Max Shepel <max@undeletable.name>

* fix: Storybook - RangeInput/Bounds example * feedback: use usEeffect for disable buttons * feedback: use usEeffect for disable buttons * fix: lint fix

britt6612 · 2024-06-06T19:45:46Z

@undeletable
Hey I was looking over this PR and Im glad to see that you caught Pagination in that it is throwing an error with not wrapped in <grommet> wrapper so I went ahead and added tests for the components on which other components also have this issue and found quite a few I was wondering if you would be open to fixing these and adding the attrs to these components
List ToggleGroup DataChart NameValueList RangeSelector WorldMap FileInput``PageHeader StarRating
These are just a few there are a few more.

#7249

undeletable · 2024-06-07T10:21:22Z

@britt6612 Pagination tests fail sometimes because of different reason (slowness), but I got your point. I'll take a look.

undeletable · 2024-06-07T10:27:56Z

@jcfilben @britt6612 I've noticed that target branch in #7249 is different from master. Should I change target branch to react-18.3.0-upgrade in my PR?

Signed-off-by: Max Shepel <max@undeletable.name>

This reverts commit 3fd3a11.

This reverts commit f492adb.

This reverts commit a954793.

undeletable · 2024-06-07T11:59:05Z

@britt6612 I've added fallbacks to default theme to all the remaining places I saw that might be needed. Tests on both my and your branch passed.

…b@users.noreply.github.com> On behalf of Basith <134603758+abdulbasithqb@users.noreply.github.com>, I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 2a4e9ac Signed-off-by: Max Shepel <max@undeletable.name>

…rchibeque@Brittanys-MBP-2.home> On behalf of Brittany Archibeque <brittanyarchibeque@Brittanys-MBP-2.home>, I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: a954793 On behalf of Brittany Archibeque <brittanyarchibeque@Brittanys-MBP-2.home>, I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: f492adb On behalf of Brittany Archibeque <brittanyarchibeque@Brittanys-MBP-2.home>, I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 3fd3a11 Signed-off-by: Max Shepel <max@undeletable.name>

…ben@users.noreply.github.com> On behalf of Jessica Filben <54560994+jcfilben@users.noreply.github.com>, I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 60d5ece Signed-off-by: Max Shepel <max@undeletable.name>

I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: a1a5a03 I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: a092b71 I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: fa50a41 Signed-off-by: Max Shepel <max@undeletable.name>

britt6612 · 2024-06-07T16:21:27Z

@jcfilben @britt6612 I've noticed that target branch in #7249 is different from master. Should I change target branch to react-18.3.0-upgrade in my PR?

@undeletable so yes what I did was make that new PR which will get merged into react-18.3.0-upgrade then we will point your PR to react-18.3.0-upgrade that way we can make sure tests are good before merging to master!

Thank you for adding this for the rest of the components

undeletable · 2024-06-07T18:32:00Z

@britt6612 I've changed base branch to react-18.3.0-upgrade, will keep the PR in sync with it

britt6612 · 2024-06-07T19:00:08Z

@undeletable thank you! Hoping my PR gets merged early next week! then we can merge yours!

undeletable added 30 commits May 1, 2024 22:19

Move default parameters from defaultProps to function definitions

cd9a5b7

Fix Bar component

14838be

Update snapshots

1766eed

Remove defaultProps from styled components - first iteration

b5e5d26

Fix styledWithTheme HOC to return correct function

f005071

Fix prototypes for styledWithTheme

9f56eca

Fix theme

019d28b

Add TODOs

45f81b2

Fix Menu test

b1ca758

Add TODO

be3a510

Remove TODO

a8ed76e

Define hook for theme context with fallback to default one

e329a7a

Pass default props to styled component

ccd1a6c

Use attrs from styled-components

87adfd3

Update snapshots

164ac90

Pass theme along with default props

3926bc9

Fix props

1abbf1b

Fix syntax

bc92022

Fix theme in props

50478dd

Merge props

9e94bce

Rename function

2911f33

Fix lint errors

780ede8

Switch from deepMerge to object copy

74b3e2b

Fix snapshots

58770f0

Try upgrading styled-components

7d584c8

Roll back to previous styled-components version

b36c664

Roll back lockfile

bd1a606

Only pass theme using attrs

86687bf

Fix snapshots

302564b

Rename method

3772235

jcfilben reviewed May 31, 2024

View reviewed changes

src/js/components/Menu/__tests__/Menu-test.js Outdated Show resolved Hide resolved

undeletable added 3 commits June 3, 2024 13:51

Add missing attrs() call. Add enhancePropsWithTheme declaration

0c1232e

Signed-off-by: Max Shepel <max@undeletable.name>

Fix Menu test

3a96b99

Signed-off-by: Max Shepel <max@undeletable.name>

Merge branch 'master' into remove_defaultProps

64e28a2

jcfilben mentioned this pull request Jun 3, 2024

Remove unnecessary snapshot from DataFilter #7248

Merged

3 tasks

undeletable and others added 5 commits June 4, 2024 13:26

Update snapshots

6820dec

Signed-off-by: Max Shepel <max@undeletable.name>

Don't mark values prop of Distribution as required as it has default …

f4ad337

…parameter value Signed-off-by: Max Shepel <max@undeletable.name>

empty commit to tty and trigger chromatic

60d5ece

fix: Storybook - RangeInput/Bounds example (grommet#7244)

2a4e9ac

* fix: Storybook - RangeInput/Bounds example * feedback: use usEeffect for disable buttons * feedback: use usEeffect for disable buttons * fix: lint fix

Merge branch 'master' into remove_defaultProps

855815c

Brittany Archibeque and others added 7 commits June 7, 2024 13:38

add tests for components outside grommet wrapper

a954793

add tests for components outside grommet wrapper

f492adb

add tests for components outside grommet wrapper

3fd3a11

Add fallback to default theme where it also might be necessary

443f0f8

Signed-off-by: Max Shepel <max@undeletable.name>

Revert "add tests for components outside grommet wrapper"

a1a5a03

This reverts commit 3fd3a11.

Revert "add tests for components outside grommet wrapper"

a092b71

This reverts commit f492adb.

Revert "add tests for components outside grommet wrapper"

fa50a41

This reverts commit a954793.

undeletable added 4 commits June 7, 2024 15:01

undeletable changed the base branch from master to react-18.3.0-upgrade June 7, 2024 18:31

undeletable mentioned this pull request Jun 7, 2024

Remove grommet wrapper #7249

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove defaultProps usages for functional components #7224

Remove defaultProps usages for functional components #7224

undeletable commented May 9, 2024 •

edited by taysea

britt6612 commented Jun 6, 2024 •

edited

undeletable commented Jun 7, 2024

undeletable commented Jun 7, 2024 •

edited

undeletable commented Jun 7, 2024

britt6612 commented Jun 7, 2024

undeletable commented Jun 7, 2024

britt6612 commented Jun 7, 2024

Remove defaultProps usages for functional components #7224

Are you sure you want to change the base?

Remove defaultProps usages for functional components #7224

Conversation

undeletable commented May 9, 2024 • edited by taysea

What does this PR do?

Where should the reviewer start?

What testing has been done on this PR?

How should this be manually tested?

Do Jest tests follow these best practices?

Any background context you want to provide?

What are the relevant issues?

Screenshots (if appropriate)

Do the grommet docs need to be updated?

Should this PR be mentioned in the release notes?

Is this change backwards compatible or is it a breaking change?

britt6612 commented Jun 6, 2024 • edited

undeletable commented Jun 7, 2024

undeletable commented Jun 7, 2024 • edited

undeletable commented Jun 7, 2024

britt6612 commented Jun 7, 2024

undeletable commented Jun 7, 2024

britt6612 commented Jun 7, 2024

undeletable commented May 9, 2024 •

edited by taysea

britt6612 commented Jun 6, 2024 •

edited

undeletable commented Jun 7, 2024 •

edited