[react-native] Correctly extend ViewProps in PressableProps#58292
Conversation
|
@eps1lon Thank you for submitting this PR! This is a live comment which I will keep updated. 1 package in this PRCode ReviewsThis PR can be merged once it's reviewed by a DT maintainer. You can test the changes of this PR in the Playground. Status
Once every item on this list is checked, I'll ask you for permission to merge and publish the changes. Diagnostic Information: What the bot saw about this PR{
"type": "info",
"now": "-",
"pr_number": 58292,
"author": "eps1lon",
"headCommitOid": "130afae7963af7492d0224f97226479cfcdb7d0c",
"mergeBaseOid": "c609479376c92be2991c6d072a70201f7e225047",
"lastPushDate": "2022-01-18T19:28:40.000Z",
"lastActivityDate": "2022-01-20T16:08:20.000Z",
"hasMergeConflict": false,
"isFirstContribution": false,
"tooManyFiles": false,
"hugeChange": false,
"popularityLevel": "Popular",
"pkgInfo": [
{
"name": "react-native",
"kind": "edit",
"files": [
{
"path": "types/react-native/index.d.ts",
"kind": "definition"
},
{
"path": "types/react-native/v0.63/index.d.ts",
"kind": "definition"
},
{
"path": "types/react-native/v0.64/index.d.ts",
"kind": "definition"
},
{
"path": "types/react-native/v0.65/index.d.ts",
"kind": "definition"
}
],
"owners": [
"alloy",
"huhuanming",
"iRoachie",
"timwangdev",
"kamal",
"alexdunne",
"swissmanu",
"bm-software",
"mvdam",
"esemesek",
"mrnickel",
"souvik-ghosh",
"nossbigg",
"saranshkataria",
"tykus160",
"jakebloom",
"ceyhun",
"mcmar",
"theohdv",
"TheSavior",
"romain-faust",
"bebebebebe",
"Naturalclar",
"chinesedfan",
"vtolochk",
"SychevSP",
"RageBill",
"sasurau4",
"256hz",
"doumart",
"drmas",
"jeremybarbet",
"ca057",
"ds300",
"natsathorn",
"connectdotz",
"TheWirv",
"alexeymolchan",
"alexbrazier",
"kuasha420",
"phvillegas",
"eps1lon"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Popular"
}
],
"reviews": [
{
"type": "approved",
"reviewer": "ca057",
"date": "2022-01-19T18:44:41.000Z",
"isMaintainer": false
}
],
"mainBotCommentID": 1015778999,
"ciResult": "pass"
} |
typescript-bot
added
Popular package
|
🔔 @alloy @huhuanming @iRoachie @timwangdev @kamal @alexdunne @swissmanu @bm-software @mvdam @Esemesek @mrnickel @souvik-ghosh @nossbigg @saranshkataria @tykus160 @jakebloom @ceyhun @mcmar @theohdv @TheSavior @romain-faust @bebebebebe @Naturalclar @chinesedfan @vtolochk @SychevSP @RageBill @sasurau4 @256hz @doumart @drmas @jeremybarbet @ca057 @ds300 @natsathorn @connectdotz @thewirv @alexeymolchan @alexbrazier @kuasha420 @phvillegas — please review this PR in the next few days. Be sure to explicitly select |
ca057
left a comment
ca057
left a comment
There was a problem hiding this comment.
I think I slowly get what this is about looking at related PRs, but I can’t match it to your PR description:
The changes all fix intersections of ReactNode with render-props. This only passed because render-props were assignable to {}.
Can you give me a hint to better connect this line to your PR?
But thanks for your work of fixing all these packages!
typescript-bot
added
the
Owner Approved
I accidentally used "intersection" when I meant "extending". An intersection would work. But extending doesn't. Does this example illustrate the issue better? interface OldViewProps {
// `{}` will be removed in React 18 types
children: string | {};
}
interface OldPressableProps extends OldViewProps {
children: () => string;
}
interface NewViewProps {
children: string;
}
interface NewPressableProps extends NewViewProps {
children: () => string;
}
|
|
Thanks for the additional explanation, now it makes sense :) |
4 participants
We plan to remove
{}fromReactFragmentsince it's not actually an allowed type for children of host components (e.g.<div>{{}}</div>would throw at runtime) (see #56026 for previous attempts).The changes all fix intersections of
ReactNodewith render-props. This only passed because render-props were assignable to{}.This change is required to pass #56210