RFC: React: Allow anything to be rendered by function components #62876

eps1lon · 2022-10-25T16:16:01Z

FYI please don't merge this yet. Want to have an extended period of time for comments.

Allows returning any from function components.

const RenderNumber = () => 42;
const RenderString = () => 'react';
const RenderArray = () => [19];

// Allowed with proposed change. Previously rejected by type-checker.
<RenderNumber />;
// Allowed with proposed change. Previously rejected by type-checker.
<RenderString />;
// Allowed with proposed change. Previously rejected by type-checker.
<RenderArray />;

In React 18 (and even more so in canary releases), there are now way more things that can be returned from render than can't be. Considering microsoft/TypeScript#14729 and microsoft/TypeScript#21699 have been open for years it seems more reasonable to disable type-checking for render return values completely.

The original thinking of TypeScript has been to be pragmatic in the things we can type-check and just give up on the edges that are commonly used at runtime but can't be (performantly) expressed in types. Return values from render seem to fit such a case.

Alternatives:

Lookup return type of factory function for JSX expression return types microsoft/TypeScript#29818 (rejected by Microsoft because of bad perf)
Decouple jsx element type from jsx factory return type and sfc return type microsoft/TypeScript#21699 (comment) (no input from Microsoft so far)
RFC: Consult new JSX.ElementType for valid JSX element types microsoft/TypeScript#51328

Not doing the ReactNode = ReactNode | Promise<ReactNode> yet since that '@next' only. Just want to test the waters with this change.

Please fill in this template.

Use a meaningful title for the pull request. Include the name of the package modified.
Test the change in your own code. (Compile and run.)
- Klarna monorepo (unused @ts-expect-error where we relied on return types being inferred)
- https://github.com/mui/material-ui/
Add or edit tests to reflect the change.
Follow the advice from the readme.
Avoid common mistakes.
Run npm test <package to test>.

Select one of these and delete the others:

If changing an existing definition:

Provide a URL to documentation or source code which provides context for the suggested changes:
- async components
- undefined from render
- closed issues
~~[ ]~~ If this PR brings the type definitions up to date with a new version of the JS library, update the version number in the header.

sebmarkbage

This is the unfortunate but pragmatic solution. It'll still catch errors on props but if you pass the element to the wrong thing or if you pass a component that returns something other than ReactNode.

It'll also still work with things that explicitly type the return value as an ReactElement which is still correct if that's all you're returning.

function Component(): React.ReactElement {
  return <div />;
}

eps1lon · 2022-10-25T17:13:30Z

It'll also still work with things that explicitly type the return value as an ReactElement which is still correct if that's all you're returning.

To be clear, the opposite is also true: Returning a JSX element from something that shouldn't would now also be allowed:

function compute(): number {
  // that would make no sense
  return <div />;
}

So codebases relying more on type inference in return values, might see a drop in type coverage.

eps1lon · 2022-10-26T08:50:24Z

types/af-utils__react-virtual-headless/af-utils__react-virtual-headless-tests.tsx

-// $ExpectType ReactElement<any, any> | null
-Subscription({ model: useVirtual(), events: [EVT_FROM], children: <>Abc</> });


Subscription is a component so this wouldn't be allowed at runtime anyway. Refactored to use JSX and removed the $ExpectType since that's testing how TypeScript behaves not the library.

eps1lon · 2022-10-26T08:50:47Z

types/carbon__icons-react/carbon__icons-react-tests.tsx

@@ -23,29 +23,29 @@ import {
    Plan,
 } from "@carbon/icons-react";

-<UserAccess />; // $ExpectType Element


Remove the $ExpectType since they're testing TypeScript not this library

DangerBotOSS · 2022-10-26T09:03:53Z

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.