RFC: Consult new JSX.ElementType for valid JSX element types #51328

eps1lon · 2022-10-27T10:40:02Z

Enables closing DefinitelyTyped/DefinitelyTyped#18051 and DefinitelyTyped/DefinitelyTyped#18912

Quick terminology intro:

  <Component />
// ^^^^^^^^^ JSX element type
//^^^^^^^^^^^^^ JSX element

Today, function components that return anything but null | JSX.Element are not allowed as element types in React. For example, const Component = () => 42; <Component /> would be rejected by the type-checker because 42 is not a valid JSX element. Note that this return value would be perfectly fine in class components.

However, in React, components can return a ReactNode. That type includes number | string | Iterable<ReactNode> | undefined (and will likely include Promise<ReactNode> in the future).

Prior attempts at fixing this issue tried to lookup the type of the JSX factory to determine what correct element types are. These attempts were abandoned due to severe perf regressions.

Instead of looking up the type of the JSX factory I'm proposing looking up the type of JSX.ElementType. This allows libraries to explicitly define what can be used as an element type. If JSX.ElementType does not exist, we fallback to the current behavior.

In React, @types/react would declare JSX.ElementType as follows (see tests for concrete usage):

// inlined `React.JSXElementConstructor`
type ReactJSXElementConstructor<Props> =
  | ((props: Props) => React.ReactNode)
  | (new (props: Props) => React.Component<Props, any>);

declare global {
  namespace JSX {
    type ElementType<Props extends object> = string | ReactJSXElementConstructor<Props>;
  }
}

Alternatives:
A. We already have a dedicated type for JSX class components. Instead of allowing to customize the whole element type, we could just introduce JSX.ElementFunction which would work similar to JSX.ElementClass
B. Alias JSX.Element to any

TODO:

polish error message
test on actual repos
perf testing on actual repos

typescript-bot · 2022-10-27T10:49:48Z

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

typescript-bot · 2022-10-27T10:56:52Z

The TypeScript team hasn't accepted the linked issue #21699. If you can get it accepted, this PR will have a better chance of being reviewed.

typescript-bot · 2022-10-27T10:56:56Z

The TypeScript team hasn't accepted the linked issue #21699. If you can get it accepted, this PR will have a better chance of being reviewed.

typescript-bot · 2022-10-27T10:56:57Z

The TypeScript team hasn't accepted the linked issue #21699. If you can get it accepted, this PR will have a better chance of being reviewed.

DanielRosenwasser · 2022-10-27T18:19:38Z

Closes #21699
Closes #14729

Is this entirely true? This allows TypeScript to customize what sorts of things can be used as tag names, but it doesn't allow users to capture the type returned by these functions as part of a JSX invocation.

To be clear, I don't have any specific objections to that direction, especially given that there's several backwards-compatibility issues and a huge performance impact involved in doing something of that sort; but I just think we need to be clear about what is/isn't getting fixed. Maybe that's pedantic. 🙂

eps1lon · 2022-10-27T18:41:38Z

I looked at the problems the issues are describing instead of the specific implementation they're proposing (especially #14729).

I have no problem in unlinking these issues (though we can always reopen the issues if people think this PR doesn't fully address their issue). I was more concerned this PR is missed/auto-closed because it didn't have an associated issue.

eps1lon · 2022-10-28T08:04:43Z

tests/baselines/reference/jsxElementType.errors.txt

+!!! related TS2728 tests/cases/compiler/jsxElementType.tsx:48:43: 'title' is declared here.
+     ~~~~~~~~~~~~~
+!!! error TS2786: 'RenderPromise' cannot be used as a JSX component.
+!!! error TS2786:   Its instance type '({ title }: { title: string; }) => Promise<string>' is not a valid JSX element.


Suggested change

!!! error TS2786: Its instance type '({ title }: { title: string; }) => Promise<string>' is not a valid JSX element.

!!! error TS2786: Its type '({ title }: { title: string; }) => Promise<string>' is not a valid JSX element type.

sounds more accurate to me.

DanielRosenwasser · 2022-10-28T20:26:24Z

From today's design meeting (#51344) we had the following questions:

Could React we get away with just modifying React.JSXElementConstructor in some fashion
- @RyanCavanaugh said the answer was "no", but maybe this is for my own understanding (and useful for future readers)
Why is the type parameter required on JSX.ElementType?

We also had a suggestion:

Could we rename JSX.ElementType to JSX.Renderable or something sufficiently different from JSX.Element? We feel like it would reduce confusion.

eps1lon · 2022-10-28T21:16:54Z

The backstory for the naming was that we already have React.ElementType. That is the type you can pass to createElement(type: ElementType).

Which is also why I copied over the type parameter. My original plan was that the type-checker uses the parameter to infer the props type but that's not implemented and I don't know if we'll ever need it. Especially considering the type parameter is ignored for host components (the string part of ElementType), we can probably remove it for now to now ship something that we don't have the full picture for.

Could we rename JSX.ElementType to JSX.Renderable or something sufficiently different from JSX.Element?

That's a bit tricky because in JSX generally and React specifically "element type" is a pretty well defined term. It's comes naturally from type Element = { type: ElementType }. Renderable specifically is too confusing since it sounds like something that can be returned from render (function components or render in class components) which ElementType certainly isn't.

We already have ElementClass so I don't understand what the need is to make it "sufficiently different".

Could React we get away with just modifying React.JSXElementConstructor in some fashion

To do what? Are you planning to lookup the React namespace in the TypeScript compiler? In #21699 specifically it was brought up that any solution should also be concerend with other libraries using JSX (see #21699 (comment)). Using the React namespace would defeat that purpose.

React.JSXElementConstructor is used as a light-weight type for components that take a specific props shape i.e. non-host components. I fear that any adjustment to make it accept props that would allow passing as host component would have perf implications since we would have to lookup JSX.IntrinsicElements. And if I remember correctly, that was always a bit costly.

Some comments on the meeting notes #51344:

Why are we revisiting?

There have been long standing issues that we can't return every possible type from function components (ReactNode) but can do so in class components: DefinitelyTyped/DefinitelyTyped#18051 and DefinitelyTyped/DefinitelyTyped#18912. So it's not just for planned features but old problems. I updated the PR description to reflect that

Will this work for class components/render methods?

It should work. However, class components are already properly typed because we can hook into ElementClass today and ensure ReactNode can be returned. And it's not yet clear if class components can have async render functions (see https://github.com/reactjs/rfcs/pull/229/files#r1008491342)

eps1lon added 2 commits Oct 27, 2022

jsx: Consult new JSX.ElementType for valid JSX element types

bf2d82e

Stricter impl of JSX.ElementType

5aee011

typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Oct 27, 2022

eps1lon marked this pull request as ready for review Oct 27, 2022

eps1lon mentioned this pull request Oct 27, 2022

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

Open

9 tasks

typescript-bot assigned weswigham Oct 27, 2022

typescript-bot added For Backlog Bug PRs that fix a backlog bug and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Oct 27, 2022

eps1lon commented Oct 28, 2022

View changes

DanielRosenwasser mentioned this pull request Oct 28, 2022

Design Meeting Notes, 10/28/2022 #51344

Open

RFC: Consult new JSX.ElementType for valid JSX element types #51328

RFC: Consult new JSX.ElementType for valid JSX element types #51328

eps1lon commented Oct 27, 2022 •

edited

typescript-bot commented Oct 27, 2022

typescript-bot commented Oct 27, 2022

typescript-bot commented Oct 27, 2022

typescript-bot commented Oct 27, 2022

DanielRosenwasser commented Oct 27, 2022 •

edited

eps1lon commented Oct 27, 2022

eps1lon Oct 28, 2022

DanielRosenwasser commented Oct 28, 2022

eps1lon commented Oct 28, 2022

	!!! error TS2786: Its instance type '({ title }: { title: string; }) => Promise<string>' is not a valid JSX element.
	!!! error TS2786: Its type '({ title }: { title: string; }) => Promise<string>' is not a valid JSX element type.

RFC: Consult new JSX.ElementType for valid JSX element types #51328

Are you sure you want to change the base?

RFC: Consult new JSX.ElementType for valid JSX element types #51328

Conversation

eps1lon commented Oct 27, 2022 • edited

typescript-bot commented Oct 27, 2022

typescript-bot commented Oct 27, 2022

typescript-bot commented Oct 27, 2022

typescript-bot commented Oct 27, 2022

DanielRosenwasser commented Oct 27, 2022 • edited

eps1lon commented Oct 27, 2022

eps1lon Oct 28, 2022

Choose a reason for hiding this comment

DanielRosenwasser commented Oct 28, 2022

eps1lon commented Oct 28, 2022

eps1lon commented Oct 27, 2022 •

edited

DanielRosenwasser commented Oct 27, 2022 •

edited