-
Notifications
You must be signed in to change notification settings - Fork 30.5k
chore(react): Add v17 infra #46690
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(react): Add v17 infra #46690
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. Status
Once every item on this list is checked, I'll ask you for permission to merge and publish the changes. InactiveThis PR has been inactive for 18 days. Diagnostic Information: What the bot saw about this PR{
"type": "info",
"now": "-",
"pr_number": 46690,
"author": "eps1lon",
"owners": [
"johnnyreilly",
"bbenezech",
"pzavolinsky",
"digiguru",
"ericanderson",
"DovydasNavickas",
"theruther4d",
"guilhermehubner",
"ferdaber",
"jrakotoharisoa",
"pascaloliv",
"hotell",
"franklixuefei",
"Jessidhia",
"saranshkataria",
"lukyth",
"eps1lon",
"zieka",
"dancerphil",
"dimitropoulos",
"disjukr",
"vhfmag",
"hellatan"
],
"dangerLevel": "ScopedAndConfiguration",
"headCommitAbbrOid": "c3063c4",
"headCommitOid": "c3063c497aa4020b08bcdff8606d1f242c76544d",
"mergeIsRequested": false,
"stalenessInDays": 18,
"lastPushDate": "2020-08-12T13:05:20.000Z",
"lastCommentDate": "2020-10-02T07:31:10.000Z",
"maintainerBlessed": true,
"reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/46690/files",
"hasMergeConflict": false,
"authorIsOwner": true,
"isFirstContribution": false,
"popularityLevel": "Critical",
"newPackages": [],
"packages": [
"react"
],
"files": [
{
"path": "types/react/OTHER_FILES.txt",
"kind": "package-meta-ok",
"package": "react"
},
{
"path": "types/react/v17/OTHER_FILES.txt",
"kind": "package-meta-ok",
"package": "react"
},
{
"path": "types/react/v17/experimental.d.ts",
"kind": "definition",
"package": "react"
},
{
"path": "types/react/v17/global.d.ts",
"kind": "definition",
"package": "react"
},
{
"path": "types/react/v17/index.d.ts",
"kind": "definition",
"package": "react"
},
{
"path": "types/react/v17/package.json",
"kind": "package-meta-ok",
"package": "react"
},
{
"path": "types/react/v17/test/cssProperties.tsx",
"kind": "test",
"package": "react"
},
{
"path": "types/react/v17/test/elementAttributes.tsx",
"kind": "test",
"package": "react"
},
{
"path": "types/react/v17/test/experimental.tsx",
"kind": "test",
"package": "react"
},
{
"path": "types/react/v17/test/hooks.tsx",
"kind": "test",
"package": "react"
},
{
"path": "types/react/v17/test/index.ts",
"kind": "test",
"package": "react"
},
{
"path": "types/react/v17/test/managedAttributes.tsx",
"kind": "test",
"package": "react"
},
{
"path": "types/react/v17/test/tsx.tsx",
"kind": "test",
"package": "react"
},
{
"path": "types/react/v17/tsconfig.json",
"kind": "package-meta",
"package": "react",
"suspect": "not the required form"
},
{
"path": "types/react/v17/tslint.json",
"kind": "package-meta",
"package": "react",
"suspect": "not the required form"
}
],
"hasDismissedReview": false,
"ciResult": "pass",
"reviewersWithStaleReviews": [
{
"reviewedAbbrOid": "0547f68",
"reviewer": "ferdaber",
"date": "2020-08-12T12:59:08Z"
}
],
"approvalFlags": 0,
"isChangesRequested": false
} |
|
🔔 @johnnyreilly @bbenezech @pzavolinsky @digiguru @ericanderson @DovydasNavickas @theruther4d @guilhermehubner @ferdaber @jrakotoharisoa @pascaloliv @Hotell @franklixuefei @Jessidhia @saranshkataria @lukyth @zieka @dancerphil @dimitropoulos @disjukr @vhfmag @hellatan — please review this PR in the next few days. Be sure to explicitly select |
|
@eps1lon The CI build failed! Please review the logs for more information. Once you've pushed the fixes, the build will automatically re-run. Thanks! |
|
@eps1lon The CI build failed! Please review the logs for more information. Once you've pushed the fixes, the build will automatically re-run. Thanks! |
Seems to not be understood by npm run test
dc76770 to
c3063c4
Compare
|
@ferdaber Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review? |
|
👋 Hi there! I’ve run some quick measurements against master and your PR. These metrics should help the humans reviewing this PR gauge whether it might negatively affect compile times or editor responsiveness for users who install these typings. Let’s review the numbers, shall we? react/v*Comparison details for react/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. react/v*
Wow, it looks like all the big movers moved in the right direction! Way to go! 🌟 I won’t post performance data again unless it gets worse. |
|
@DanielRosenwasser Is this approach problematic for the infra in this repo? We started some conversation but it fizzled out (https://twitter.com/drosenwasser/status/1293702990105161729). |
|
Not abandoned |
|
Superseded by #48971 |
Copies current typings into
v17-rcat which we'll target upcoming breaking changes.Discussion about breaking changes should happen in #46691. This PR is only for discussion how we release
@types/react@17.If we move the current typings into v16 and let
react/index.d.tspoint to v17 typings then the installed types from@types/reactwould be 17 whilereactwould point to 16. So we need to put 17 typings in a separate file/folder. The question is what approach the types publisher understands:Alternatively we use the same approach as
experimental.d.tsand addv17-rc.d.tsbut it's unclear ifdtslintwould understand that this file would run with a different typescript version(planned for v17). This is not planned for React 17 but we want to do it for 18 and with this PR we can check if this approach is viable.