refactor: use transient props in various styled components #5093
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
| focus: boolean | ||
| hover: boolean |
There was a problem hiding this comment.
These were removed as the styled component wasn't doing anything with these values
| onMouseEnter={handleMouseEnter} | ||
| onMouseLeave={handleMouseLeave} |
There was a problem hiding this comment.
Similarly, this internal state has been removed as ChangeBarWrapper isn't varying on the value of hover
| // We exclude `documentId` from spread props to avoid passing it to the Card component | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| documentId, |
There was a problem hiding this comment.
Would love to know if there's a more elegant way to do this!
There was a problem hiding this comment.
I think you should be able to assingn it to _ and have eslint ignore it, e.g. documentId: _
There was a problem hiding this comment.
I considered this! Though I believe we need to amend our eslint config slightly to accommodate, e.g.
'@typescript-eslint/no-unused-vars': [
'warn',
{
varsIgnorePattern: '^_',
},
],
Happy to add if you feel appropriate! Otherwise we can just leave as-is
There was a problem hiding this comment.
Since const {foo: _, bar: _, ...rest} = … will yell at you for redeclaring consts, will that rule allow omitting more than one property? Maybe we could configure it to be _ignore_${propertyName} (or maybe that's too much?).
There's also no-unused-vars#ignorerestsiblings, which is more specific to this particular case.
| content={content} | ||
| // data-placement={restProps.placement} | ||
| portal | ||
| allowedAutoPlacements={['top', 'bottom']} |
There was a problem hiding this comment.
Removed as it's now deprecated in Sanity UI, and including it would yield similar transient prop warnings
|
No changes to documentation |
robinpyon
requested review from
a team
and removed request for
bjoerge and
RitaDias
|
Component Testing Report Updated Oct 30, 2023 6:16 PM (UTC)
|
| // We exclude `documentId` from spread props to avoid passing it to the Card component | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| documentId, |
There was a problem hiding this comment.
I think you should be able to assingn it to _ and have eslint ignore it, e.g. documentId: _
Description
This PR migrates a number of styled components to now use transient props, and remove a number of warnings that were shown in localhost as a result of upgrading to styled-components to
6.x.A fully exhaustive audit wasn't conducted, but this should address most of the more common / egregious examples.
What to review
Touched components should be unaffected in function or style. We should no longer see a glut of transient prop related warnings in localhost.
Notes for release
N/A