-
Notifications
You must be signed in to change notification settings - Fork 53
BREAKING: add render*
props for each shorthand prop
#328
Conversation
Codecov Report
@@ Coverage Diff @@
## master #328 +/- ##
==========================================
+ Coverage 89.32% 89.55% +0.23%
==========================================
Files 62 62
Lines 1180 1216 +36
Branches 176 178 +2
==========================================
+ Hits 1054 1089 +35
- Misses 124 125 +1
Partials 2 2
Continue to review full report at Codecov.
|
render*
props for each shorthand proprender*
props for each shorthand prop
Component: React.ReactType, | ||
props: IProps, | ||
children: ReactChildren, | ||
) => React.ReactElement<any> |
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.
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.
Nice. We were just discussing improving typings for shorthand in Stardust as well.
Note on SUIR's typings here:
React.ComponentType
only includes Component and SFC but not strings ('div'
, etc). UseReact.ReactType
to include all valid component types.React.ReactChildren
is actually the API for theReact.Children
utility. UseReact.ReactNodeArray | React.ReactNode
for return values from render functions.
73b98fa
to
7b485a7
Compare
ProblemsThere are several problems with the approach provided, though - let me mention them. 1. Points of general confusion for client
// lets just provide a list of urls :)
<Menu items={[ 'http://..', .. ]} renderItem={.. process URL and render tree ..} It means that in some cases
<Menu items={[
{ key: ..., ... }
...
]}
renderItem={(Component, props) => ... where items data comes to? .. } 2. "Do It Yourself" pattern provided by
|
Before
Shorthand used to accept a render function:
After
Dedicated
render*
props have been added for each shorthand prop:This allows shorthand data to be passed and processed while simultaneously allowing a custom render tree of that preprocessed data.
Currently, it is not possible to define both shorthand data and a render function for components that take an array of shorthand. The array elements are used as values for the shorthand. Passing a render function for a given shorthand array element means that array element now cannot be a value, such as a props object.
What we need is a way to iterate over the shorthand items with a render function. This way, the factory can still normalize the shorthand values to
Component
andprops
, the parent component can compute state, styling, and accessibility, and the user gets to construct the tree using all this information:This format gives consumers the ability to simultaneously:
This pattern needs to apply to all our components and I have done so in this PR.