-
Notifications
You must be signed in to change notification settings - Fork 130
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 Pagination component return type #1635
Conversation
Hey @robinmetral, Thanks! |
🦋 Changeset detectedLatest commit: d250162 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov Report
@@ Coverage Diff @@
## main #1635 +/- ##
=======================================
Coverage 91.54% 91.54%
=======================================
Files 195 195
Lines 4080 4080
Branches 1296 1296
=======================================
Hits 3735 3735
Misses 326 326
Partials 19 19
|
'@sumup/circuit-ui': patch | ||
--- | ||
|
||
Changed `Pagination` type from `ReactNode` to `ReactElement | null` to prevent a clash with React 18 types. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed `Pagination` type from `ReactNode` to `ReactElement | null` to prevent a clash with React 18 types. | |
Changed `Pagination` return type from `ReactNode` to `ReactElement | null` to prevent a clash with React 18 types. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aaah, Kodiak was too fast 😅
Purpose
Currently, the
Pagination
component is typed as aReactNode
.This can be a problem for apps using React 18, because
ReactNode
can beundefined
, andundefined
isn't a valid JSX component.Approach and changes
Switch the return type to
ReactElement | null
.This is more accurate, since the
Pagination
component never returns e.g.undefined
, a string, or an array ofReactElement
s (all included in the definition ofReactNode
).It can either be
null
(when there are less than two pages) or aReactElement
otherwise.Definition of done
Unit and integration testsMeets minimum browser supportMeets accessibility requirements