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

[eps1lon]

Closes #21699
Closes #14729

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 = string | ReactJSXElementConstructor<any>;
  }
}

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

benchmarks

Klarna monorepo

Some compat issues uncovered for components being declared as ForwardRefRenderFunction. This is fine since you're not supposed to use these as element types but rather pass them to forwardRef.

diff (sample size=1)

Identifiers:                +80
Symbols:                    -8757
Types:                      -12359
Instantiations:             -72126
Memory used:                -303298K
Assignability cache size:   +4800
Identity cache size:        +17
Subtype cache size:         +381
Strict subtype cache size:  -710

reference

Files:                        40741
Lines of Library:             28610
Lines of Definitions:        370226
Lines of TypeScript:        1716268
Lines of JavaScript:              0
Lines of JSON:               531661
Lines of Other:                   0
Identifiers:                2567179
Symbols:                    5811368
Types:                      2233827
Instantiations:            23142335
Memory used:               5839695K
Assignability cache size:   1222270
Identity cache size:         106410
Subtype cache size:           55075
Strict subtype cache size:    84573
I/O Read time:                9.15s
Parse time:                  10.09s
ResolveModule time:          13.66s
ResolveTypeReference time:    0.11s
Program time:                37.85s
Bind time:                    4.89s
Check time:                  97.73s
printTime time:               0.01s
Emit time:                    0.02s
Total time:                 140.49s

baseline

Files:                        40741
Lines of Library:             28675
Lines of Definitions:        370220
Lines of TypeScript:        1716268
Lines of JavaScript:              0
Lines of JSON:               531661
Lines of Other:                   0
Identifiers:                2567099
Symbols:                    5820125
Types:                      2246186
Instantiations:            23214461
Memory used:               6142993K
Assignability cache size:   1217470
Identity cache size:         106393
Subtype cache size:           54694
Strict subtype cache size:    85283
I/O Read time:               18.61s
Parse time:                  11.21s
ResolveModule time:          17.55s
ResolveTypeReference time:    0.11s
Program time:                53.10s
Bind time:                    4.02s
Check time:                 169.17s
printTime time:               0.02s
Emit time:                    0.03s
Total time:                 226.32s

[typescript-bot]

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]

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]

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]

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]

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]

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.

[DanielRosenwasser]

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]

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)

[Andarist]

Would this solve an issue with "exotic components" (return values from React.forwardRef, React.memo, etc)? I think that it wouldn't but I'm not sure. Perhaps it could allow them to be used as element types but if I understand correctly it wouldn't provide type-checking for their props.

The issue that I refer to is the fact that those components add a fake callable signature to their types in @types/react to satisfy the type checker. This signature doesn't actually exist at runtime and it even comes with such a note:

Can a simple alternative design be found here to allow such "exotic components"? I know that @types/react won't remove those fake callable signatures anytime soon from its types but it would be nice if this wouldn't be mandated by newer TS versions.

[eps1lon]

Can a simple alternative design be found here to allow such "exotic components"?

I thought about it and I'm not sure yet we actually want to properly type these. It may even be more useful to make element types fully opaque since you're not supposed to deal with their structure anyway (e.g. calling a function component directly is really bad since it makes assumptions about their implementation).

It may make more sense for things that are symbols and these cases can be solved by the proposed ElementType. Props inference would be unsolved at the moment though but the original plan was to use the type parameter for that. Either way, exotic components are not in the scope of this PR since there's no apparent issue at the moment.

[Andarist]

Either way, exotic components are not in the scope of this PR since there's no apparent issue at the moment.

IMHO the issue is there though. Sure, it's not the end of the world - but those exotic components shouldn't be callable at the type level. The added JSDoc description is nice but it just shouldn't be needed. I get that might be puristic of me - but this is how I see it and from my PoV, it is a problem (that exists at the moment).

I get the problem that you are trying to solve here but at the same time, this is an excellent moment to attempt to solve both problems at the same time, together. I would just like to avoid a scenario in which this PR gets merged in without considering this other problem. If eventually this other problem would get solved, it feels quite likely to me (I could be wrong) that the fix for it would make this thing here redundant - and from that perspective, I think that it's at least worth trying to fix both at once now.

I agree that the types of those exotic components, or function components, could even be more opaque (although I doubt that you'd like to mandate annotating all function components with some kind of a special type to make them opaque at the declaration sites). But that would still require some type of checker support to be added and for this to be considered in the JSX design in the compiler

[eps1lon]

IMHO the issue is there though. Sure, it's not the end of the world - but those exotic components shouldn't be callable at the type level. The added JSDoc description is nice but it just shouldn't be needed.

How would that be different from adding correct types for something like forwardRef though? Now instead of a function you have an object with a render method. Neither of these things should be exposed to the user. But we could only fix it with opaque types which TypeScript doesn't have.

I get that might be puristic of me - but this is how I see it and from my PoV, it is a problem (that exists at the moment).

What is the problem that exists today? It sounds like you're saying people calling forwardRef components directly today and running into runtime type issues? Personally I have never seen that while I do see plenty of people running into problems with render return values frequently. This proposal focuses on the common issues I've seen.

I get the problem that you are trying to solve here but at the same time, this is an excellent moment to attempt to solve both problems at the same time, together.

Do you have some concrete ideas that you want to share?

[DanielRosenwasser]

(FWIW the "Why are we revisiting?" is sometimes posed as a motivating question in the design meeting to provide with "what's different also?", but I appreciate that background @eps1lon)

[Andarist]

We could offload the decision/mapping of what is a valid type to the framework. We already have LibraryManagedAttributes - so we could have LibraryAllowedElementTypes:

type LibraryAllowedElementTypes<T> = T extends ExoticComponent<infer Props> ? Props : never

I'm not saying that this is the exact API that I'm proposing - but something along those lines would probably be an option. To allow what you want to allow here you could expand this to:

type LibraryAllowedElementTypes<T> = T extends ReactJSXElementConstructor<
  infer Props
>
  ? Props
  : T extends ExoticComponent<infer Props>
  ? Props
  : string;

We'd have to figure out the exact rules for how this would fit the existing types in the JSX namespace etc but all of that seems to be doable.

What is the problem that exists today? It sounds like you're seeing people calling forwardRef components directly today and running into runtime type issues? Personally I have never seen that while I do see plenty of people running into problems with render return values frequently. This proposal focuses on the common issues I've seen.

I'm not saying that this is a practical problem - it's still a design problem in my opinion though. Clearly, JSX in TypeScript has not been designed to allow arbitrary element types in the past. it was designed for the React needs at the time - and it shows. That's fine but JSX was adopted by more libraries, React evolves, allows more things etc. So I think that this is a great opportunity to rethink how the JSX implementation in TS could become more flexible to allow what you are proposing and fix some old limitations at the same time.

[nickserv]

Are we planning on handling the rendering of Promises here, or should we handle it separately?

[eps1lon]

Are we planning on handling the rendering of Promises here, or should we handle it separately?

This PR enables handling that. We can't do it without this PR. But we also don't want to do it here. Since JSX is a general concept and there's no spec that libraries using JSX need to be able to render Promises. As far as I know only React server components can.

[eps1lon]

@sandersn I'm not sure what's expected of me now. I'm waiting for a reply to #51328 (comment). Unless I misunderstood @DanielRosenwasser and this wasn't a question but a definitive statement:

• remove the Props type parameter
• rename to JSX.Renderable

Or should I implement looking up the Props type parameter by the type-checker so that we can fully type out exotic components as requested by @Andarist? Even though no library using JSX has conveyed interest in that yet.

[TylorS]

@DanielRosenwasser Yes, I've been hoping to switch from using tagged template literals (https://github.com/TylorS/typed-fp/blob/development/packages/html/src/tag/Fx.ts#L16) to utilizing JSX to gain an easier path to supporting type-safe properties (without language service plugins and an additional type checker), and access to the JSX AST for compiler optimization, while still being able to aggregate typed resources and typed errors in unions from Effect-ts (formerly fp-ts) to build composable components.

For a pretty basic example, https://github.com/TylorS/typed-fp/blob/development/example/pages/layout.ts#L15, the returned type understand that a Link component, and also an outlet, require a Router service to function.

[urakymzhan]

Anyone know when ElementType will be available in @types/react?

[unstubbable]

Anyone know when ElementType will be available in @types/react?

See DefinitelyTyped/DefinitelyTyped#65135 (and DefinitelyTyped/DefinitelyTyped#65220)

[eps1lon]

Plan is to ship once TypeScript 5.1 stable out. May ship after they cut the RC if I get too excited.

[pkellner]

Sorry for a little off the topic, but does anyone know if there are version numbers I can put in my package.json that will allow for this function signature error not to happen in my code. Here is my current package.json

I don't need it to be safe for production, just development so I can do screen recordings for a course without having the error showing in VS-Code

{
  "name": "nextapp",
  "version": "0.1.0",
  "private": true,
  "scripts": {
    "dev": "next dev",
    "build": "next build",
    "start": "next start",
    "lint": "next lint"
  },
  "dependencies": {
    "@types/node": "^18.16.3",
    "@types/react": "^18.2.0",
    "@types/react-dom": "^18.2.0",
    "bootstrap": "^5.2.3",
    "eslint": "^8.39.0",
    "eslint-config-next": "^13.3.2",
    "next": "^13.3.2",
    "react": "18.2.0",
    "react-dom": "18.2.0",
    "sass": "^1.60.0",
    "server-only": "^0.0.1",
    "typescript": "^5.0.4"
  }
}

[DanielRosenwasser]

Hey @pkellner, just use the nightly (npm install -D typescript@next). If you need a specific version, you can use typescript@5.1.0-dev.20230501.

There's also a VS Code nightly extension for TypeScript/JavaScript: https://marketplace.visualstudio.com/items?itemName=ms-vscode.vscode-typescript-next

[pkellner]

@DanielRosenwasser I've been trying that and similar and still showing the error (which is a problem for my instructional video I'm doing, as it looks like an error but really isn't).

Here are my steps that I think should eliminate the error.

1. go to empty folder (on Mac M2 though I don't think that matters)
2. enter command: npx create-next-app@latest --experimental-app. (accept all default prompts making sure to say Y to typescript)
3. Bring up app in VSCode opening my-app folder, do everything thru vscode terminal prompt now
4. npm install;npm run dev (from my-app folder)
5. verify app works at localhost:3000
6. In /my-app/src/app folder create a new async component named my-comp.tsx and put code below in it.
7. update /src/app/page.tsx component to be just 5 lines (put code below in it that calls MyComp component
8. Verify error MyComp cannot be used as a JSX component.... (see screen shot below for error)
9. Try @DanielRosenwasser fix above, at terminal prompt, enter the command npm install -D typescript@next
10. Notice it gives no errors, but still the react JSX error like is shown below is there
11. running npm install gives version mismatch error so need to run npm install -f
12. run npm run dev again
13. Still have JSX error

/src/app/page.tsx

import MyComp from './my-comp'

export default function Home() {
  return <MyComp />
}

/src/app/my-comp.tsx

export default async function MyComp() {
    return <div>MyComp</div>
}

Current package.json after update

{
  "name": "my-app",
  "version": "0.1.0",
  "private": true,
  "scripts": {
    "dev": "next dev",
    "build": "next build",
    "start": "next start",
    "lint": "next lint"
  },
  "dependencies": {
    "@types/node": "18.16.3",
    "@types/react": "18.2.0",
    "@types/react-dom": "18.2.1",
    "eslint": "8.39.0",
    "eslint-config-next": "13.3.4",
    "next": "13.3.4",
    "react": "18.2.0",
    "react-dom": "18.2.0"
  },
  "devDependencies": {
    "typescript": "^5.1.0-dev.20230501"
  }
}

Error we have always had for async components and still have after @DanielRosenwasser fix

[jakebailey]

@pkellner

Per #51328 (comment) and #51328 (comment) above, I do not believe the changes needed in @types/react have been released, so there is not a way to test it without manually applying the linked PRs.

[pkellner]

Thanks @jakebailey, is there a straight forward way for someone like me (who is good at git, but not a wizard) create my own TypeScript and run it in VS Code? Otherwise, my alternative is to dim the error to almost nothing and hope people don't see it in my upcoming Pluralsight React Server Components course. I can't completely turn off errors because then I don't see them as I'm typing and it will drive me nuts. I have done a ton of docs on docs.microsoft.com so I'm not without skills, but like I said, I'm not a wizard like you folks.

[jakebailey]

If you really need it ASAP and aren't concerned about incompleteness, then the easiest way would be to use something like patch-package and apply the changes from the linked @types/react PRs for your project, then use typescript@next. But, it feels a little weird to suggest so if the behavior is going to be set in stone via some recorded course when this is all still a work in progress and not fully merged, not to mention unavailable to those who don't also patch their packages (until 5.1).

[pkellner]

My contract is such that I don't release the course until NextJS 13 experimental releases, and I can't imagine that they would release before this is fixed. Even if they do release, I've got at least 6 more weeks of work to do my 3 hour course (I'm an hour done, but mostly with soft demos, no vs-code yet). So, bottom line, I'm gambling a little, but not that much I don't think. I'm counting on 6 plus weeks from now, the code I'm showing will not cause that error when people install nextjs 13, react, and the current TypeScript. I definitely will not release a patchy type process on a course teaching released s/w.

Would you mind giving me a few more details on the steps and versions I need to install. I'd really apprecate it. It will get my demo's to look good instead of red squiggles all over the place. Re-recording all that would be really hard. It literally takes me about 1 month per hour of finished video including all the recording and edits (I'm slow, but pretty much that's the same for all the other authors at Pluralsight I respect the most).

Thanks again, and sorry for the big ask. it's important to me.

[jakebailey]

In short, if you're using react, you have @types/react installed. You can use patch-package locally and apply DefinitelyTyped/DefinitelyTyped#65135 and DefinitelyTyped/DefinitelyTyped#65220 to your installed package.