[react] Add support for Promise in ReactNode in experimental release channel
#65220
Conversation
af7a12a to
39779e5
Compare
|
@eps1lon Thank you for submitting this PR! This is a live comment which I will keep updated. 1 package in this PRCode 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": 65220,
"author": "eps1lon",
"headCommitOid": "ec1371b0d96b6ae6ab1b773f4b7d7898a8aee16b",
"mergeBaseOid": "f3d00387558f6a5cbf6f94bc6123e8d1836a1d72",
"lastPushDate": "2023-05-04T05:32:55.000Z",
"lastActivityDate": "2023-05-04T05:32:55.000Z",
"hasMergeConflict": false,
"isFirstContribution": false,
"tooManyFiles": false,
"hugeChange": false,
"popularityLevel": "Critical",
"pkgInfo": [
{
"name": "react",
"kind": "edit",
"files": [
{
"path": "types/react/experimental.d.ts",
"kind": "definition"
},
{
"path": "types/react/index.d.ts",
"kind": "definition"
},
{
"path": "types/react/test/experimental.tsx",
"kind": "test"
},
{
"path": "types/react/test/index.ts",
"kind": "test"
},
{
"path": "types/react/test/tsx.tsx",
"kind": "test"
},
{
"path": "types/react/ts5.0/experimental.d.ts",
"kind": "definition"
},
{
"path": "types/react/ts5.0/index.d.ts",
"kind": "definition"
},
{
"path": "types/react/ts5.0/test/experimental.tsx",
"kind": "test"
},
{
"path": "types/react/ts5.0/test/index.ts",
"kind": "test"
},
{
"path": "types/react/ts5.0/test/tsx.tsx",
"kind": "test"
}
],
"owners": [
"johnnyreilly",
"bbenezech",
"pzavolinsky",
"ericanderson",
"DovydasNavickas",
"theruther4d",
"guilhermehubner",
"ferdaber",
"jrakotoharisoa",
"pascaloliv",
"hotell",
"franklixuefei",
"Jessidhia",
"saranshkataria",
"lukyth",
"eps1lon",
"zieka",
"dancerphil",
"dimitropoulos",
"disjukr",
"vhfmag",
"hellatan",
"priyanshurav",
"Semigradsky"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Critical"
}
],
"reviews": [],
"mainBotCommentID": 1516654024,
"ciResult": "pass"
} |
|
🔔 @johnnyreilly @bbenezech @pzavolinsky @ericanderson @DovydasNavickas @theruther4d @guilhermehubner @ferdaber @jrakotoharisoa @pascaloliv @Hotell @franklixuefei @Jessidhia @saranshkataria @lukyth @zieka @dancerphil @dimitropoulos @disjukr @vhfmag @hellatan @priyanshurav @Semigradsky — 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! Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review. |
39779e5 to
af7a12a
Compare
|
@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! Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review. |
af7a12a to
39779e5
Compare
|
@eps1lon Unfortunately, this pull request currently has a merge conflict 😥. Please update your PR branch to be up-to-date with respect to master. Have a nice day! |
39779e5 to
33294b2
Compare
|
@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! Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review. |
33294b2 to
61040e9
Compare
|
Inspecting the JavaScript source for this package found some properties that are not in the .d.ts files. react (unpkg)was missing the following properties:
|
61040e9 to
757f812
Compare
Every test run expects the opposite of what's in $ExpectType. I've never seen this and have now idea how this could happen. Considering prop-types matching against TS types is highly exotic and prop-types doesn't support promises as React nodes to begin with, I'm skipping these for now. Haven't seen a PR to prop-types in ages anyway so they probably "just" work.
Problem is that now only native Promise or anything implementing the global PromiseLike explicitly are accepted. Any custom thenable that just implements `then` will be rejected. So now you're incentivized to use the public "DO_NOT_USE_*" symbol for your custom thenables which kinda defeats the whole "DO_NOT_USE_*" naming. So bundling types with facebook/react it is I guess?
It's a bit unfortunate that it has its own name that's surface to devs. I hope it looks close enough to `PromiseLike<ReactNode>` that people understand what it means.
757f812 to
310d92a
Compare
|
Out for 2 weeks, integrates well into Klarna codebase. Shipping to unblock #65135 |
This does not add support for async function components. Async function components need this PR and #65135.
It does add support for passing promises to children (e.g.
<div>{Promise.resolve('React')}</div>and effectively returning promises from class components if you opted into React's experimental types (see top comment ofreact/experimental.d.tsfor instructions).We can't differentiate between async render methods, render methods that return a new promise or render methods that return a promise they got from somewhere else at the type level. We'd need a linter for that.
The implementation is a bit hacky so that we can continue to leverage module augmentation. This is important since we can only ship types as
@latestfrom DT.Users then have to include a single
/// <reference types="react/experimental" />in their compilation (other methods explained in the top comment ofreact/experimental.d.ts). Frameworks using experimental release channels should use one of the methods above so that their users automatically get types for experimental release channels. Latest releases of Next.js already does that.