-
Notifications
You must be signed in to change notification settings - Fork 53
fix(factories): shorthand fix for applying props to react element #496
Conversation
Codecov Report
@@ Coverage Diff @@
## master #496 +/- ##
=========================================
Coverage ? 88.32%
=========================================
Files ? 42
Lines ? 1456
Branches ? 212
=========================================
Hits ? 1286
Misses ? 165
Partials ? 5 Continue to review full report at Codecov.
|
3fc758c
to
595c817
Compare
docs/src/examples/components/Input/Variations/InputExampleWrapperSlot.shorthand.tsx
Show resolved
Hide resolved
595c817
to
8798832
Compare
8798832
to
c73cc84
Compare
c73cc84
to
32ffd52
Compare
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.
Can we check my solution for areTypeNamesEqual
? I think that the check of displayName
is not the best solution.
marking blocked until having a resolution on: |
f1d4047
to
9af4c9e
Compare
docs/src/examples/components/RadioGroup/Item/RadioGroupItemExampleChecked.shorthand.tsx
Outdated
Show resolved
Hide resolved
docs/src/examples/components/RadioGroup/Types/RadioGroupColorPickerExample.shorthand.tsx
Show resolved
Hide resolved
ece34d1
to
6714a85
Compare
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.
We discussed these changes before, the final result LGTM 👍
docs/src/examples/components/RadioGroup/Item/RadioGroupItemExample.shorthand.tsx
Show resolved
Hide resolved
6714a85
to
e76bf82
Compare
createShorthandFactory
fix for applying props to React ElementDescription
This PR tackles the first argument type of the function returned by
createShorthandFactory
factory, more precisely when passing aReact.Element
as argument.This comes into play in examples such as:
that translate into calling:
and rendering:
The
icon
argument can be anyReact.Element
so we cannot make too many presumptions on what to do with the props send as the 2nd argument (defaultProps
andoverrideProps
).Current Behavior:
We blindly send all props to the

React.cloneElement
call (seecreateShorthand
call infactories.tsx
).For the example above, this can lead to passing invalid props to certain components and we currently catch this wrong usage in one of our unit tests:
Solution:
If the first argument of the function returned by
createShorthandFactory
factory is aReact.Element
just return the element early without cloning and applying extra props.