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.

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!

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

jcfilben

Looks good!

britt6612 · 2024-06-13T18:50:17Z

src/js/components/Diagram/Diagram.js

-  const [dimensions, setDimensions] = useState({ width: 0, height: 0 });
-  const [connectionPoints, setConnectionPoints] = useState();
-  const svgRef = useForwardedRef(ref);
+const defaultConnectionsValue = [];


why was this added?

It's added in order to keep parity with defaultProps approach and avoid potentially redundant rerenders or even rerender loops.
Let's consider a prop with value of object type. Assume it has no value passed and falls back to value from defaultProps. In this case, the value would be equal between renders (props.prop === defaultProps.prop), since defaultProps is static.
If we replace defaultProps with default function parameters, and the default value of mentioned prop is defined as object literal, this means that on each render its values would be different. On each render component function is called again, so default parameter value is re-evaluated. If component state depends on such prop (e.g., via useEffect or useMemo), this would result in infinite rerenders loop.
That's why I've initialized such props with stable links. Actually this approach was already used in some other places of grommet.

Thank you ☝️

britt6612 · 2024-06-13T18:53:16Z

src/js/components/Distribution/propTypes.js

@@ -43,7 +43,7 @@ if (process.env.NODE_ENV !== 'production') {
      PropTypes.shape({
        value: PropTypes.number,
      }),
-    ).isRequired,


britt6612 · 2024-06-13T18:57:30Z

src/js/components/Grid/StyledGrid.js

@@ -216,6 +216,7 @@ const areasStyle = (props) => {
 const StyledGrid = styled.div
  .withConfig(styledComponentsConfig)
  .attrs((props) => ({
+    ...enhancePropsWithTheme(props),


why is this one formatted different just trying to understand

Because attrs was already used for passing aria-label (see the line below). So using attrs(enhancePropsWithTheme) would result in losing that property.

oh gotcha thank you for clarifying

britt6612 · 2024-06-13T19:28:11Z

Thank you @undeletable for all your help! we have a few more things to add but this was a big help!

* Remove grommet wrapper (#7249) * re-trigger token circle ci * add tests for components outside grommet wrapper * add tests for components outside grommet wrapper * add tests for components outside grommet wrapper * Update src/js/components/Calendar/__tests__/Calendar-test.tsx Co-authored-by: Jessica Filben <54560994+jcfilben@users.noreply.github.com> * Update src/js/components/FormField/__tests__/FormField-test.js Co-authored-by: Jessica Filben <54560994+jcfilben@users.noreply.github.com> * Update src/js/components/ThumbsRating/__tests__/ThumbsRating.tsx Co-authored-by: Jessica Filben <54560994+jcfilben@users.noreply.github.com> * remove data friends tests * remove data friends tests * update tests * update tests --------- Co-authored-by: Brittany Archibeque <brittanyarchibeque@Brittanys-MBP-2.home> Co-authored-by: Jessica Filben <54560994+jcfilben@users.noreply.github.com> * Remove defaultProps usages for functional components (#7224) * Move default parameters from defaultProps to function definitions * Fix Bar component * Update snapshots * Remove defaultProps from styled components - first iteration * Fix styledWithTheme HOC to return correct function * Fix prototypes for styledWithTheme * Fix theme * Add TODOs * Fix Menu test * Add TODO * Remove TODO * Define hook for theme context with fallback to default one * Pass default props to styled component * Use attrs from styled-components * Update snapshots * Pass theme along with default props * Fix props * Fix syntax * Fix theme in props * Merge props * Rename function * Fix lint errors * Switch from deepMerge to object copy * Fix snapshots * Try upgrading styled-components * Roll back to previous styled-components version * Roll back lockfile * Only pass theme using attrs * Fix snapshots * Rename method * Add comment. DCO Remediation Commit for Max Shepel <max@undeletable.name> I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: cd9a5b7 I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 14838be I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 1766eed I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: b5e5d26 I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: f005071 I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 9f56eca I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 019d28b I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 45f81b2 I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: b1ca758 I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: be3a510 I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: a8ed76e I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: e329a7a I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: ccd1a6c I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 87adfd3 I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 164ac90 I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 3926bc9 I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 1abbf1b I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: bc92022 I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 50478dd I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 9e94bce I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 2911f33 I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 780ede8 I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 74b3e2b I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 58770f0 I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 7d584c8 I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: b36c664 I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: bd1a606 I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 86687bf I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 302564b I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 3772235 Signed-off-by: Max Shepel <max@undeletable.name> * Update snapshots * Remove 'only' from test * Remove attrs() usages where it's not required * DCO Remediation Commit for Max Shepel <max@undeletable.name> I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 6cc1019 I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 429627c I, Max Shepel <max@undeletable.name>, hereby add my Signed-off-by to this commit: 5c80f67 Signed-off-by: Max Shepel <max@undeletable.name> * Fix typo Signed-off-by: Max Shepel <max@undeletable.name> * Add missing attrs() call. Add enhancePropsWithTheme declaration Signed-off-by: Max Shepel <max@undeletable.name> * Fix Menu test Signed-off-by: Max Shepel <max@undeletable.name> * Don't mark values prop of Distribution as required as it has default parameter value Signed-off-by: Max Shepel <max@undeletable.name> * empty commit to tty and trigger chromatic * fix: Storybook - RangeInput/Bounds example (#7244) * fix: Storybook - RangeInput/Bounds example * feedback: use usEeffect for disable buttons * feedback: use usEeffect for disable buttons * fix: lint fix * add tests for components outside grommet wrapper * add tests for components outside grommet wrapper * add tests for components outside grommet wrapper * Add fallback to default theme where it also might be necessary Signed-off-by: Max Shepel <max@undeletable.name> * Revert "add tests for components outside grommet wrapper" This reverts commit 3fd3a11. * Revert "add tests for components outside grommet wrapper" This reverts commit f492adb. * Revert "add tests for components outside grommet wrapper" This reverts commit a954793. * Third-Party DCO Remediation Commit for Basith <134603758+abdulbasithqb@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> * Third-Party DCO Remediation Commit for Brittany Archibeque <brittanyarchibeque@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> * Third-Party DCO Remediation Commit for Jessica Filben <54560994+jcfilben@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> * DCO Remediation Commit for 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> * Add missing snapshots Signed-off-by: Max Shepel <max@undeletable.name> --------- Signed-off-by: Max Shepel <max@undeletable.name> Co-authored-by: Jessica Filben <54560994+jcfilben@users.noreply.github.com> Co-authored-by: Basith <134603758+abdulbasithqb@users.noreply.github.com> Co-authored-by: Brittany Archibeque <brittanyarchibeque@Brittanys-MBP-2.home> * Remove isRequired from propTypes (#7252) * remove isRequired from propTypes * revert unnecessary removals of isRequired * add isrequired to fromTarget and toTarget and link diagram proptypes to the diagram component * refactor routed components for react 19 compatibility (#7253) * Upgrade react version * rename function to withTheme * incorporate review comments --------- Signed-off-by: Max Shepel <max@undeletable.name> Co-authored-by: Brittany <42451602+britt6612@users.noreply.github.com> Co-authored-by: Brittany Archibeque <brittanyarchibeque@Brittanys-MBP-2.home> Co-authored-by: Max Shepel <undeletable@users.noreply.github.com> Co-authored-by: Basith <134603758+abdulbasithqb@users.noreply.github.com>

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

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

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

Merged

3 tasks

jcfilben mentioned this pull request Jun 12, 2024

Remove isRequired from propTypes #7252

Merged

3 tasks

jcfilben and others added 2 commits June 12, 2024 15:57

Merge branch 'react-18.3.0-upgrade' into remove_defaultProps

145e758

Add missing snapshots

4b4ba1f

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

jcfilben approved these changes Jun 12, 2024

View reviewed changes

britt6612 reviewed Jun 13, 2024

View reviewed changes

britt6612 self-requested a review June 13, 2024 19:20

britt6612 approved these changes Jun 13, 2024

View reviewed changes

britt6612 merged commit b8179c8 into grommet:react-18.3.0-upgrade Jun 13, 2024

undeletable deleted the remove_defaultProps branch June 13, 2024 19:29

jcfilben mentioned this pull request Jun 18, 2024

React 18.3.0 upgrade #7257

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Remove defaultProps usages for functional components #7224

Remove defaultProps usages for functional components #7224

Uh oh!

undeletable commented May 9, 2024 •

edited by taysea

Loading

Uh oh!

undeletable commented Jun 7, 2024

Uh oh!

britt6612 commented Jun 7, 2024

Uh oh!

undeletable commented Jun 7, 2024

Uh oh!

britt6612 commented Jun 7, 2024

Uh oh!

jcfilben left a comment

Uh oh!

britt6612 Jun 13, 2024

Uh oh!

undeletable Jun 13, 2024

Uh oh!

britt6612 Jun 13, 2024

Uh oh!

britt6612 Jun 13, 2024

Uh oh!

britt6612 Jun 13, 2024

Uh oh!

undeletable Jun 13, 2024

Uh oh!

britt6612 Jun 13, 2024

Uh oh!

britt6612 commented Jun 13, 2024

Uh oh!

Uh oh!

Remove defaultProps usages for functional components #7224

Remove defaultProps usages for functional components #7224

Uh oh!

Conversation

undeletable commented May 9, 2024 • edited by taysea Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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?

Uh oh!

undeletable commented Jun 7, 2024

Uh oh!

britt6612 commented Jun 7, 2024

Uh oh!

undeletable commented Jun 7, 2024

Uh oh!

britt6612 commented Jun 7, 2024

Uh oh!

jcfilben left a comment

Choose a reason for hiding this comment

Uh oh!

britt6612 Jun 13, 2024

Choose a reason for hiding this comment

Uh oh!

undeletable Jun 13, 2024

Choose a reason for hiding this comment

Uh oh!

britt6612 Jun 13, 2024

Choose a reason for hiding this comment

Uh oh!

britt6612 Jun 13, 2024

Choose a reason for hiding this comment

Uh oh!

britt6612 Jun 13, 2024

Choose a reason for hiding this comment

Uh oh!

undeletable Jun 13, 2024

Choose a reason for hiding this comment

Uh oh!

britt6612 Jun 13, 2024

Choose a reason for hiding this comment

Uh oh!

britt6612 commented Jun 13, 2024

Uh oh!

Uh oh!

undeletable commented May 9, 2024 •

edited by taysea

Loading