fix(ListItem): align Content and Title prop types with what they render#4034
Open
patrickwehbe wants to merge 1 commit into
Open
Conversation
ListItem.Content renders a View but its props extended TextProps, so View props were rejected by the type checker while Text-only props were accepted and silently ignored at runtime. Change ListItemContentProps to extend ViewProps. ListItem.Title renders the rneui Text component but its props extended react-native's TextProps, so the extra rneui props (h1-h4, h1Style, ...) could not be passed. Change ListItemTitleProps to extend the rneui ../Text TextProps. Note: extending ViewProps instead of TextProps on ListItem.Content is a breaking change to that component's public types. Fixes react-native-elements#3893
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Fixes #3893.
Two
ListItemsubcomponents declare prop types that don't match the element they render, so following the docs for extending the theme'sTextPropsproduces TypeScript errors.ListItem.Contentrenders a<View>(and its JSDoc says "Receives all View props"), butListItemContentPropsextendedTextProps. Text-only props were accepted by the type checker and then silently dropped at runtime by theView, while realViewprops were rejected.ListItem.Titlerenders the rneui<Text>component, butListItemTitlePropsextended react-native'sTextPropsinstead of rneui's../TextTextProps. As a result the rneui-specific props (h1–h4,h1Style, ...) and any user-extended themedTextPropscould not be passed toListItem.Title, even thoughListItem.Contentaccepted them.This matches the report in the issue: the extra props ended up usable on
Content(where they do nothing) but not onTitle(where they belong).Changes
ListItem.Content.tsx:ListItemContentPropsnow extendsViewProps(fromreact-native).ListItem.Title.tsx:ListItemTitlePropsnow extendsTextPropsfrom../Text(rneui) instead ofreact-native.Breaking change
Changing
ListItem.ContentfromTextPropstoViewPropsis a breaking change to that component's public prop types. Code that passed Text-only props toListItem.Contentwill now get a type error. In practice those props were ignored at runtime already (the component renders aView), so this only surfaces an existing mismatch, but it is a type-level breaking change and I wanted to call it out explicitly in case you'd prefer to land it with the next major. TheTitlechange is additive (it widens the accepted props), not breaking.I kept the diff intentionally minimal.
ListItem.Subtitlehas the same react-native-vs-rneuiTextPropsmismatch asTitle; I left it out to keep this PR focused, but I'm happy to include it here or in a follow-up if you'd like.Type of change
ListItem.ContentonlyTest Plan
yarn installyarn typescript(tsc --noEmit) passes on the full repo with the fix applied.tscfail (e.g.'h1' does not exist in type 'ListItemTitleProps'), confirming the fix is what resolves the mismatch.