Skip to content

chore: migrate defaultProps to default assignment #2008

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

chore: migrate defaultProps to default assignment #2008

wants to merge 1 commit into from

Conversation

chris-schneider-zen
Copy link

Description

As of React v19 defaultProps is fully deprecated. Currently in React v18 it does warn against this being removed, so we can easily migrate these within the codebase.

This PR does the work of eliminating and replace defaultProps with one of two patterns:
a. default parameter values (with destructuring)
b. nullish coalescing within styled::attrs() callbacks

Details

@@ -21,9 +21,9 @@ const AccordionComponent = forwardRef<HTMLDivElement, IAccordionProps>(
       children,
       isBare,
       isCompact,
-      isAnimated,
+      isAnimated = true,
       isExpandable,
-      isCollapsible,
+      isCollapsible = true,
       level,
       onChange,
       defaultExpandedSections,
@@ -103,11 +103,6 @@ const AccordionComponent = forwardRef<HTMLDivElement, IAccordionProps>(
 
 AccordionComponent.displayName = 'Accordion';
 
-AccordionComponent.defaultProps = {
-  isAnimated: true,
-  isCollapsible: true
-};
-

Checklist

  • 👌 design updates will be Garden Designer approved (add the designer as a reviewer)
  • 🌐 demo is up-to-date (npm start)
  • ⬅️ renders as expected with reversed (RTL) direction
  • ⚫ renders as expected in dark mode
  • 🤘 renders as expected with Bedrock CSS (?bedrock)
  • 💂‍♂️ includes new unit tests. Maintain existing coverage (always >= 96%)
  • ♿ tested for WCAG 2.1 AA accessibility compliance
  • 📝 tested in Chrome, Firefox, Safari, and Edge

jzempel
jzempel reviewed Jul 25, 2025
View reviewed changes
Copy link
Member

@jzempel jzempel left a comment

Choose a reason for hiding this comment

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

@ chris-schneider-zen thanks so much for your patience on this. The update is looking great. I left a few comments. Along with addressing those, if you're able to make a couple of updates, I think we'll be able to get this through fairly quickly:

  • resolve conflicts with the current main branch
  • if possible, move this to a direct branch rather than a fork (I sent you an org invite). That will make sure the full CI runs, including deployment. And that will make my turnaround review much easier.

🙇

packages/grid/src/styled/StyledCol.ts
})<IStyledColProps>`
'data-garden-version': PACKAGE_VERSION,
$columns: props.$columns ?? 12,
theme: props.theme ?? DEFAULT_THEME
Copy link
Member

Choose a reason for hiding this comment

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

Due to https://github.com/zendeskgarden/react-components/blob/main/docs/adrs/002-theme-default-prop-removal.md, we can remove this

packages/grid/src/styled/StyledGrid.ts
})<IStyledGridProps>`
'data-garden-version': PACKAGE_VERSION,
$gutters: props.$gutters ?? 'md',
theme: props.theme ?? DEFAULT_THEME
Copy link
Member

Choose a reason for hiding this comment

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

Ditto, this theme default can be removed

packages/grid/src/styled/StyledRow.ts
})<IStyledRowProps>`
'data-garden-version': PACKAGE_VERSION,
$wrapAll: props.$wrapAll ?? 'wrap',
theme: props.theme ?? DEFAULT_THEME
Copy link
Member

Choose a reason for hiding this comment

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

Ditto, this theme default can be removed

packages/tags/src/styled/StyledTag.ts
'data-garden-version': PACKAGE_VERSION
})<IStyledTagProps>`
'data-garden-version': PACKAGE_VERSION,
theme: props.theme ?? DEFAULT_THEME,
Copy link
Member

Choose a reason for hiding this comment

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

Ditto, this theme default can be removed

packages/typography/src/elements/Paragraph.tsx
<StyledParagraph ref={ref} {...props} />
));
export const Paragraph = forwardRef<HTMLParagraphElement, IParagraphProps>(
({ size = 'medium', ...props }, ref) => <StyledParagraph ref={ref} size={size} {...props} />
Copy link
Member

Choose a reason for hiding this comment

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

Not imperative for this PR, but it would be good to fix the $size transient prop naming for the underlying StyledParagraph component

packages/typography/src/styled/StyledCode.ts
$isMonospace: true
})<IStyledCodeProps>`
$isMonospace: true,
theme: props.theme || DEFAULT_THEME,
Copy link
Member

Choose a reason for hiding this comment

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

Ditto, this theme default can be removed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jzempel jzempel jzempel left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chris-schneider-zen @jzempel