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

Spread default icon props #2615

Merged
merged 3 commits into from
Jul 24, 2024
Merged

Spread default icon props #2615

merged 3 commits into from
Jul 24, 2024

Conversation

connor-baer
Copy link
Member

Purpose

defaultProps has been deprecated on function components in favor of default parameters in React 18.3 (facebook/react#25699). A warning message is logged for every component that uses defaultProps which creates a lot of noise and hides other important console messages.

We use https://github.com/airbnb/babel-plugin-inline-react-svg/ to transform the SVG files into React components. Historically, the plugin statically assigned default props as defaultProps. There's a 4-year-old pull request to add an option to spread props instead (airbnb/babel-plugin-inline-react-svg#86). Unfortunately, it hasn't been merged yet despite repeated prompting.

I don't want to fork the library since we have no capacity to maintain it and I still hold out hope that the patch will be eventually merged 🤞

Approach and changes

  • Vendor babel-plugin-inline-react-svg with the above patch applied

Definition of done

  • Development completed
  • Reviewers assigned
  • Unit and integration tests
  • Meets minimum browser support
  • Meets accessibility requirements

@connor-baer connor-baer requested a review from a team as a code owner July 24, 2024 16:17
Copy link

vercel bot commented Jul 24, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
oss-circuit-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 24, 2024 4:17pm

Copy link

changeset-bot bot commented Jul 24, 2024

🦋 Changeset detected

Latest commit: 64e1de3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@sumup/icons Major
@sumup/astro-template-circuit-ui Patch
@sumup/circuit-ui Major
@sumup/remix-template-circuit-ui Patch
next-app Patch
@sumup/eslint-plugin-circuit-ui Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

codecov bot commented Jul 24, 2024

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 86.58%. Comparing base (325e4a2) to head (64e1de3).
Report is 1 commits behind head on next.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             next    #2615      +/-   ##
==========================================
- Coverage   86.73%   86.58%   -0.15%     
==========================================
  Files         209      209              
  Lines       11902    11907       +5     
  Branches     1531     1499      -32     
==========================================
- Hits        10323    10310      -13     
- Misses       1526     1544      +18     
  Partials       53       53              
Files Coverage Δ
packages/icons/scripts/build.ts 0.00% <0.00%> (ø)

... and 11 files with indirect coverage changes

@connor-baer connor-baer merged commit 985f647 into next Jul 24, 2024
11 of 14 checks passed
@connor-baer connor-baer deleted the fix/default-props-warning branch July 24, 2024 16:22
@connor-baer connor-baer mentioned this pull request Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant