[react] Allow returning ReactNode from function components

[eps1lon]

Note: This change only applies to TypeScript 5.1 and later

Adds a new ElementType under the JSX namespace that is used by TypeScript 5.1 to determine if an element type is valid. This will allow function components, forwardRef components etc to return ReactNode (strings, arrays, numbers, boolean, undefined) from render. This won't change anything for class components since they already worked as intended.

For example:

async function Component() {
  return null;
}

// Would error but doesn't anymore with this change
<Component />;

async function Layout({ children }) {
  return children;
}

// Would error but doesn't anymore with this change
<Layout />;

Closes #18051
Closes #18912
Closes #20249
Closes #20356
Closes #20544
Closes #20942
Closes #26890
Closes #33006
Closes #33908
Closes #41808
Closes #25349
Closes #23422

for reviewers

This PR will not land before 5.1.2 is published (scheduled for 30th May). It's up for review early (probably once RC is out) to ship it as soon as possible to avoid the "why hasn't this been fixed yet" crowd.

`20a2f4db4f` N=1 --extendedDiagnostics diff

 Files:                        43577
 Lines of Library:             37989
-Lines of Definitions:        346337
+Lines of Definitions:        346349
-Lines of TypeScript:        1907773
+Lines of TypeScript:        1907763
 Lines of JavaScript:              0
 Lines of JSON:               563522
 Lines of Other:                   0
-Identifiers:                2741434
+Identifiers:                2741435
-Symbols:                    6350962
+Symbols:                    6339383
-Types:                      2447541
+Types:                      2436031
-Instantiations:            28166165
+Instantiations:            28140173
-Memory used:               6588764K
+Memory used:               6574798K
-Assignability cache size:   1116038
+Assignability cache size:   1122063
-Identity cache size:         110625
+Identity cache size:         110731
-Subtype cache size:           56507
+Subtype cache size:           56510
-Strict subtype cache size:   108885
+Strict subtype cache size:   108283

[DangerBotOSS]

Inspecting the JavaScript source for this package found some properties that are not in the .d.ts files.
The check for missing properties isn't always right, so take this list as advice, not a requirement.

react (unpkg)

was missing the following properties:

1. unstable_act

Generated by 🚫 dangerJS against 68a39e5

[eps1lon]

We test this in elementTypeTests now

[lukeapage]

I tested out the new types w/ the new typescript to see how it looked on our nearly 1 million l.o.c. react repo and the only thing broken was one weird case with story books ComponentStory type, in a single case where

const Y = (props: {}) => (<div/>);

const X: ComponentStory<typeof Y> = () => 'x';

const Z = () => (<X/>);

which gives

'X' cannot be used as a JSX component.
  Its type 'ComponentStory<(props: {}) => Element>' is not a valid JSX element type.
    Type 'ComponentStory<(props: {}) => Element>' is not assignable to type '(props: any) => ReactNode'.
      Target signature provides too few arguments. Expected 2 or more, but got 1.

which seems to be because

export declare type ArgsStoryFn<TFramework extends AnyFramework = AnyFramework, TArgs = Args> = (args: TArgs, context: StoryContext<TFramework, TArgs>) => TFramework['storyResult'];

e.g. the legacy context api argument

If I add this in to the JSXElementConstructor type then the error goes away

    type JSXElementConstructor<P> =
        | ((props: P, context?: any) => ReactNode)
        | (new (props: P) => Component<any, any>);

Thanks for the PR and all the work that went into it - I'm not an expert, maybe this is intentional, just posting my analysis in case it is useful.

[eps1lon]

Thanks for the PR and all the work that went into it - I'm not an expert, maybe this is intentional, just posting my analysis in case it is useful.

Thank you very much for testing this change.

This sounds like another case I also encountered in some NPM package for React Native. I need to double check what the state of the second parameter of function components is. But I'm leaning towards not supporting it since it makes forwardRef render functions ambigious.

I'll definitely update the PR description to mention this potential breakage.

[typescript-bot]

@eps1lon Thank you for submitting this PR!

This is a live comment which I will keep updated.

1 package in this PR

• react — on npm, on unpkg (author is owner)
  • owner-approval: @johnnyreilly

Code Reviews

Because 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

• ✅ No merge conflicts
• ✅ Continuous integration tests have passed
• ✅ Most recent commit is approved by a DT maintainer

All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes.

---

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "now": "-",
  "pr_number": 65135,
  "author": "eps1lon",
  "headCommitOid": "68a39e5b4fdc23d964324c4b06af8820d855fa15",
  "mergeBaseOid": "7e0c6ff9e446423a052e8bc23ac74b5ace1f6990",
  "lastPushDate": "2023-05-28T08:57:35.000Z",
  "lastActivityDate": "2023-06-01T19:25:12.000Z",
  "mergeOfferDate": "2023-05-28T09:29:52.000Z",
  "mergeRequestDate": "2023-06-01T19:25:12.000Z",
  "mergeRequestUser": "eps1lon",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "react",
      "kind": "edit",
      "files": [
        {
          "path": "types/react/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react/jsx-dev-runtime.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react/jsx-runtime.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"
        }
      ],
      "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": [
    {
      "type": "approved",
      "reviewer": "johnnyreilly",
      "date": "2023-05-28T09:00:19.000Z",
      "isMaintainer": true
    }
  ],
  "mainBotCommentID": 1557836847,
  "ciResult": "pass"
}

[eps1lon]

Ready to merge

[mnpenner]

Adds a new ElementType under the JSX namespace that is used by TypeScript 5.1 to determine if a element type is valid. This will allow function components, forwardRef components etc to return ReactNode (strings, arrays, numbers, boolean, undefined) from render.

Is undefined actually allowed? Null should be but not undefined. That helps catch bugs when you forget to return something.

[nickmccurdy]

Yes, React has supported components returning undefined for a bit.

[eps1lon]

Is undefined actually allowed? Null should be but not undefined. That helps catch bugs when you forget to return something.

undefined is allowed as of React 18.

No return is currently not allowed i.e. when you for forgot to put a return statement in your render body, the type-checker will raise an error since "'void' is not assignable to ReactNode". We do have tests for catching no return

DefinitelyTyped/types/react/test/tsx.tsx

Lines 707 to 715 in e221cd2

	// Desired behavior.
	// @ts-expect-error
	<ReturnVoid />;
	// @ts-expect-error
	React.createElement(ReturnVoid);
	// @ts-expect-error
	<RenderVoid />;
	// @ts-expect-error
	React.createElement(RenderVoid);

.

[sisiea]

Type error is still reported when I use async component.

Here is my code.

import React from 'react';

const Foo = async () => {
    return <div>asd</div>;
};

const Com = () => {
    return <Foo />
}

package.json

{
  "name": "typescript-demo",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "tsc": "tsc -v && tsc",
    "tsct": "tsc --traceResolution",
    "tscv": "tsc -v"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "devDependencies": {
    "@types/react": "^18.2.8",
    "typescript": "^5.1.3"
  },
  "repository": "https://github.com/sisiea/build-dev-tools-action.git",
  "dependencies": {
    "react": "^18.2.0"
  }
}

@type/react version is 18.2.8

[nickmccurdy]

@sisiea I'm not getting any TypeScript errors with the following config:

{
  "compilerOptions": {
    "esModuleInterop": true,
    "jsx": "react-jsx"
  }
}

If this doesn't fix it, please either push to the repository and ping me or share your config here for help.

[jplwood]

@sisiea No clue if this is any help, but I had a similar issue, my nextjs app had latest typescript (5.2.2) and @types/react (18.2.8) and I was still seeing the type error.

In my case, I'm using VS Code and my app is in a monorepo (using NPM workspaces + Turborepo). Turns out I had another version of typescript installed that VS Code was using. Once I selected the 5.2.2 version, my IDE type errors went away (cmd+shift+P -> TypeScript: Select TypeScript Version -> choose the correct version)