Fix mergeProp
, createElemPropsHook
, and composeHooks
types
#2979
NicholasBoll
started this conversation in
General
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
mergeProps
mergeProps
is a utility function used by all of our components to properly merge React props fromReact hooks and the props you pass to components. The functionality merges props like the
cs
prop,css
prop,style
prop, callbacks, and other props in a way that tries to be intuitive. If you adda
cs
prop and a hook also has acs
prop, both are merged in such a way that thecs
prop fromboth is represented. Same with the
style
prop - the style objects will be merged together.Callbacks are merged in a way that both callbacks are called. The strange and undocumented merging
was when one of the props is
null
. Basically, in the case ofmergeProps(props1, props2)
,props2
will overrideprops1
(callbacks are merged so thatprops2
callbacks are called afterprops1
), except ifprops1
sets anull
value.The
null
processing allows behavior hooks to "remove" props from passed props. Usually this is forprop redirection or allow passing non-element props to a hook and the hook can remove it. The
runtime works fine, but TypeScript didn't like how we merged the types:
The problem is if a prop has a
null
and a non-null value:TypeScript Playground
There is also a problem if a prop type was changed:
TypeScript Playground
Since we used
&
, an intersection type, there is no intersection betweenstring
andnumber
andtherefore
foo
becomesnever
. The runtime actually makesfoo
be anumber
.Fixing this revealed a common problem - a prop type may require a narrowed type, but a hook provides a widened type. This required us to add
as const
to our props:We use the TypeScript 5.0 const Type Parameters to narrow the type. The object passed is a constant, so we upgraded
mergeProps
andcreateElemPropsHook
to use const Type Parameters to automatically narrow the type to avoid this issue.We upgraded the type of
mergeProps
to be more complicated, but more accurate so that prop types override prop types and also thenull
handle works:createElemPropsHook
The
createElemPropsHook
now uses const Type Parameters to narrow types. Similar tomergeProps
:We use const Type Parameters to narrow the type since the elemProps hook returns a read-only object. This prevents you from needing to provide
as const
.composeHooks
composeHooks
composes multiple hooks together, callingmergeProps
along the way. If we used anull
in any hook the result was returningnever
. We updated to accurately return the correct type. This could be alerting you to errors you didn't see before since the spreadelemProps
didn't have props before and thus wouldn't have any conflicts.Beta Was this translation helpful? Give feedback.
All reactions