[react-native] Add id prop to Text, TouchableWithoutFeedback and View components#62019
[react-native] Add id prop to Text, TouchableWithoutFeedback and View components#62019gabrieldonadel wants to merge 1 commit intoDefinitelyTyped:masterfrom
Conversation
|
@gabrieldonadel Thank you for submitting this PR! This is a live comment which I will keep updated. 1 package in this PR
Code ReviewsBecause this is a widely-used package, a DT maintainer will need to review it before it can be merged. 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": 62019,
"author": "gabrieldonadel",
"headCommitOid": "098db8493a033e29df6b99485c35f73f1e23d81c",
"mergeBaseOid": "2c4c1d0b106a1305c384cde84bb14bbf1a938d2c",
"lastPushDate": "2022-08-30T01:12:07.000Z",
"lastActivityDate": "2022-09-19T19:54:19.000Z",
"hasMergeConflict": false,
"isFirstContribution": false,
"tooManyFiles": false,
"hugeChange": false,
"popularityLevel": "Critical",
"pkgInfo": [
{
"name": "react-native",
"kind": "edit",
"files": [
{
"path": "types/react-native/index.d.ts",
"kind": "definition"
},
{
"path": "types/react-native/test/index.tsx",
"kind": "test"
}
],
"owners": [
"alloy",
"huhuanming",
"iRoachie",
"timwangdev",
"kamal",
"alexdunne",
"swissmanu",
"bm-software",
"mvdam",
"esemesek",
"mrnickel",
"souvik-ghosh",
"nossbigg",
"saranshkataria",
"tykus160",
"jakebloom",
"ceyhun",
"mcmar",
"theohdv",
"romain-faust",
"bebebebebe",
"Naturalclar",
"chinesedfan",
"vtolochk",
"SychevSP",
"RageBill",
"sasurau4",
"256hz",
"doumart",
"drmas",
"jeremybarbet",
"ds300",
"natsathorn",
"connectdotz",
"alexeymolchan",
"alexbrazier",
"kuasha420",
"phvillegas",
"eps1lon",
"ZihanChen-MSFT",
"kelset",
"MateWW"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Critical"
}
],
"reviews": [
{
"type": "changereq",
"reviewer": "eps1lon",
"date": "2022-09-19T19:54:19.000Z"
},
{
"type": "approved",
"reviewer": "kuasha420",
"date": "2022-09-19T14:42:43.000Z",
"isMaintainer": false
},
{
"type": "approved",
"reviewer": "iRoachie",
"date": "2022-09-19T13:02:15.000Z",
"isMaintainer": false
}
],
"mainBotCommentID": 1230961227,
"ciResult": "pass"
} |
|
🔔 @alloy @huhuanming @iRoachie @timwangdev @kamal @alexdunne @swissmanu @bm-software @mvdam @Esemesek @mrnickel @souvik-ghosh @nossbigg @saranshkataria @tykus160 @jakebloom @ceyhun @mcmar @theohdv @romain-faust @bebebebebe @Naturalclar @chinesedfan @vtolochk @SychevSP @RageBill @sasurau4 @256hz @doumart @drmas @jeremybarbet @ds300 @natsathorn @connectdotz @alexeymolchan @alexbrazier @kuasha420 @phvillegas @eps1lon @ZihanChen-MSFT @kelset @MateWW — please review this PR in the next few days. Be sure to explicitly select |
|
@gabrieldonadel The CI build failed! Please review the logs for more information. Once you've pushed the fixes, the build will automatically re-run. Thanks! Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review. |
4c986ea to
098db84
Compare
|
Am I correct in assuming this is a PR for an unreleased feature? If that's a case, could you convert this to a draft so that it doesn't create more notifications considering we can't merge it yet? |
Sure, this PR is related to facebook/react-native#34522, I'm converting this back to a draft as you suggested then |
|
It has been more than two weeks and this PR still has no reviews. I'll bump it to the DT maintainer queue. Thank you for your patience, @gabrieldonadel. (Ping @alloy, @huhuanming, @timwangdev, @kamal, @alexdunne, @swissmanu, @bm-software, @mvdam, @Esemesek, @mrnickel, @souvik-ghosh, @nossbigg, @saranshkataria, @tykus160, @jakebloom, @ceyhun, @mcmar, @theohdv, @romain-faust, @bebebebebe, @Naturalclar, @chinesedfan, @vtolochk, @SychevSP, @RageBill, @sasurau4, @256hz, @doumart, @drmas, @jeremybarbet, @ds300, @natsathorn, @connectdotz, @alexeymolchan, @alexbrazier, @phvillegas, @eps1lon, @ZihanChen-MSFT, @kelset, @MateWW.) |
|
@eps1lon facebook/react-native#34522 has been merged and 0.70 release, I belive this will be available in the next release |
|
Thanks @gabrieldonadel for adding this! Now that we've copied these types over to react-native (https://github.com/facebook/react-native/blob/main/types/index.d.ts), I'll copy your changes over and from now on we can just submit PRs to the react-native repo |
There was a problem hiding this comment.
The merge commit for the linked PR is not tagged with any release yet. It was also merged 10 days ago while 0.70 was released 14 days ago. 0.70.1 release notes make no mention of this change. reactnative.dev docs also do not mention this prop.
The types in this repository reflect the documented and released API. Not the head of main in facebook/react-native.
Until the linked change is release, this should not be merged. I suggest going back to draft and re-requesting review once it's released.
|
@gabrieldonadel One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits. Thank you! |
Summary: Changelog: [Internal] - Apply changes from recent `id` property adds. These are changes by gabrieldonadel from DefinitelyTyped/DefinitelyTyped#62019 Reviewed By: cipolleschi Differential Revision: D39646408 fbshipit-source-id: d75f2e2b5b3d157825c4aaf21775e0d5165383ee
Please fill in this template.
npm test <package to test>.This PR is the follow-up of facebook/react-native#34522, and add types for the new
idprop to theImage,Text,TouchableWithoutFeedbackandViewcomponents. It also adds new tests for theidprop.