Decouple jsx element type from jsx factory return type and sfc return type

[weswigham]

We need to look up the type of a jsx expression by actually resolving the jsx factory call, so that we don't create a reference to the global JSX.Element type, which can change shape between react versions (as it needs to in the react 16 upgrade). We also need to resolve the sfc return type and class element type from the parameters of the factory function overloads for the same reasons, doubly so because the types allowable as render method and SFC return values are no longer the same as JSX.Element (namely, they can be strings, arrays, portals, etc).

This might be considered a breaking change, because some consumers may expect JSX.Element to always be a supertype of both jsx element expression return types and SFC return types (even though this isn't true in react 16) - we certainly made that assumption internally, hence the need for the change. 🐱

[aaronjensen]

@weswigham would addressing this address https://github.com/Microsoft/TypeScript/issues/18357 ? It'd be great to be able to correctly type children render props in React.

[jchitel]

There are a lot of issues related to this that have been closed, saying that this issue will handle them. I am assuming that the case in #18357 will be handled by this as well.

At the core of this is the ability to type JSX based off of the createElement function, rather than using the set of interfaces in the JSX namespace. If that feature is indeed going to be implemented as part of this issue, then a natural extension would be that the children could end up anywhere in the resulting element, and they could be typed based off of anything, rather than just restricting children to the props.

[ericanderson]

@jchitel agreed. I just hit this snag trying to change the react typings yesterday for this exact same reason.

[ericanderson]

@weswigham If you haven't started on this yet, I'd like to take a crack at it

[weswigham]

@ericanderson I actually started work on this today, sorry 😉

[ericanderson]

@weswigham Damn. Can you make sure I can capture the SFC or ComponentClass so we can do things like limit the children of <ButtonGroup> to <Button>?

[weswigham]

@ericanderson That's the plan. (I mean, technically it was a bug report a long time ago that we closed as "fixed" even though it was only half-fixed)

[ericanderson]

I think that will also let us mark the defaultProps optional.

[weswigham]

By defaultProps do you mean the intrinsic props?

[ericanderson]

I mean:

interface Props {
  foo: string,
  bar: number
}
class Foo extends React.Component<Props> {
  static defaultProps = { foo: "Hi Mom!" };
//...
}

In this world, the compiler should only require me to specify bar and foo should now be considered foo?: string

[weswigham]

... maybe. Even with localized types looked up from the signatuers, we still have to make assumptions about how the custom ctor/sfc is effectively built, because there's actually two calls going on. The first call is the constructor for the class or the SFC, (which are expected to take props as an argument), and can be overloaded - this is where a lot of props validation actually happens. The second call is the factory function itself, which actually needs the reified types from the inner call to be typechecked correctly (And today isn't actually typechecked at all, which is the root of most of these issues).

TBQH a lot of the JSX machinery in-place today is to get higher-ordery behavior from a system that used to not support any. I think now that we have conditional and infer types, there's a good chance I may actually be able to desugar a lot of the magic currently applied to JSX to normal typesystem operations. I'm generally just going to look at improving this as much as is feasible 😉

[ericanderson]

So the props we should enforce are actually Omit<Props, typeof Foo.defaultProps> & Partial<typeof Foo.defaultProps>... which maybe we can't do with this change.

[ericanderson]

@weswigham Thanks!

[DanielRosenwasser]

I'm not entirely sure that this was technically fixed. #18131 was fixed, but we still don't actually look at what a given JSX factory invocation returns.

[weswigham]

Correct.

[dyst5422]

What are the technical impediments to doing ReturnType<factoryFunction> in typechecking JSX?

...or do I have absolutely no idea how any of this stuff works.

[DanielRosenwasser]

ReturnType doesn't use overload resolution, nor does it apply type argument inference, both which utilize the actual props passed in to a JSX element.

So you'd potentially get an "under-instantiated" and/or broad type depending on if your JSX factory is generic or has overloads (or both).

[dyst5422]

Oh yeah, now I remember reading that caveat to ReturnType<>. Bummer.

[bcherny]

This improvement still won't allow recursively enforcing type constraints on JSX children, right? The use case I'm thinking of is React's new Context API.

// good
render() {
  let {Consumer, Provider} = React.createContext(undefined)
  return <Provider value={{a: 1}}>
    <Consumer>
      {state => state.a}
    </Consumer>
  </Provider>
}

// bad
render() {
  let {Consumer, Provider} = React.createContext(undefined)
  return <Consumer>
    {state =>
      <div>
        <Provider value={{a: 1}}>
          {state.a} // runtime error
        </Provider>
      </div>
    }
  </Consumer>
}

[ShanonJackson]

This has been an issue for so long. When will we get a fix for this? My issue is similar to those described in the thread with typings children of ButtonGroup to only take Button. My issue is that i have RenderProp components that enhance elements however they break when passed a Component instance instead of a native JSX.Element and there is no way to type against it currently.

a Component instance I.E <ButtonGroup/> is inherently different to say a native jsx element <div/> they have different properties and different behaviors and we need a way to type them accordingly.

• type on is a function type on native is a string.
• refs and event bindings behave differently when passed to one or the other.
• ReactDOM behaves differently when passed one or the other.
• refs when applied behave differently consider ref.getBoundingClientRect(); being called on a ref of a component instance vs a native jsx element.

I heard rumors this would be fixed with typescript 2.8, and then 2.9 but still nothing. see comments for https://stackoverflow.com/questions/49269743/protect-against-react-instances with references to github.com/Microsoft/TypeScript/issues/21699

[SergioMorchon]

Just to clarify, I understand that this issue is expected to allow us to define, for example, something like flowtype's https://flow.org/en/docs/react/children/#toc-only-allowing-a-specific-element-type-as-children

//… from flowtype's documentation
type Props = {
  children: React.ChildrenArray<React.Element<typeof TabBarIOSItem>>,
};

Am I right?

This feature could be a really big deal to use TypeScript in our Project (made with JS + React, looking for type-check it), because we have a lof of JSX components that we would like to type-check its children elements, and currently it is not posible to do with TypeScript.

[weswigham]

Yes, technically. You can already define such a type in TS, it just isn't terribly useful since every JSXExpression is erased to the base JSXElement.

[dead-claudia]

Related: https://github.com/Microsoft/TypeScript/issues/14789

[aleclarson]

Does this fix the following problem?

let foo = React.createElement(Foo)
foo = <Foo />
/* [ts]
Type 'Element' is not assignable to type 'ComponentElement<{}, Foo>'.
  Types of property 'type' are incompatible.
    Type 'string | ComponentClass<any, any> | StatelessComponent<any>' is not assignable to type 'ComponentClass<{}, any>'.
      Type 'string' is not assignable to type 'ComponentClass<{}, any>'. */

If not, does the above problem have a fix in the works?

I expect <Foo /> to return a React.ComponentElement<Foo['props'], Foo> value, not a JSX.Element value. At least when I'm using "jsx": "react" in tsconfig.json.

Maybe I can work around this by editing/overriding the exports of @types/react?

[weswigham]

@aleclarson Yes, this would be the change which would hopefully allow that.

[aleclarson]

@weswigham Awesome!

I actually started work on this today, sorry 😉

Is it still a high priority for you? 😎

edit: Oh, looks like it's in-progress still. Can't wait!

[weswigham]

Yeah, we got a lot of the way where with the new LibraryManagedAttributes entrypoint we added for react last release; but that just fixes up the props, the return type we have yet to manage.

[AlexGalays]

I'm yet another person who tried to fix the Functional Component types of react to be more open (and reflect what react can actually handle) only to be met with cryptic errors because of this issue ^^

this should compile indeed:

[Jessidhia]

Yeah, unfortunately right now when returning children you need to cast them to as JSX.Element | null 😢

[ljharb]

@Kovensky or self-recursive array, or string, or number, I believe, as of react 16.

[Jessidhia]

You can return them, you just have to cast them to as JSX.Element | null in current typescript, possibly with an as any in the middle. This is because TS had a hardcoded internal type for function components. I think TS@next fixes this but I haven't checked.

[danielkcz]

What I did when I encountered this issue was to just create ambient declaration react.d.ts and I am using only that everywhere. If it gets fixed someday, I can just change these to point to the real thing.

declare type ReactNode = string | number | React.ReactElement<any> | null
declare type ChildrenNode = ReactNode | ReactNode[]

Btw, with hooks it won't be needed as much because render prop won't be that cool anymore 😂

[AlexGalays]

What I did when I encountered this issue was to just create ambient declaration react.d.ts and I am using only that everywhere. If it gets fixed someday, I can just change these to point to the real thing.

declare type ReactNode = string | number | React.ReactElement<any> | null
declare type ChildrenNode = ReactNode | ReactNode[]

Btw, with hooks it won't be needed as much because render prop won't be that cool anymore

I still need it for a component that returns its previous or current children and animate them; it also use hooks (like usePrevious)

I didn't get how your custom type declaration for react helps this issue?

[danielkcz]

Well, it helps for a several reasons:

1) There is no undefined as in the original so I don't get runtime issue from that 2) Since the any is neatly tucked in that file, I can use no-any rule just fine 3) It's ambient, so I don't need to worry about importing it all the time :P

Had no issues with this so far.

[dead-claudia]

@weswigham Do you have any status update on this?

[Jessidhia]

To be able to do a real fix for this we'd probably need JSX.Element to be generic instead of an any-widened ReactElement.

Speaking of which, I should see what happens if I change the return type of FunctionComponent in 3.2.

[bluepnume]

Weighing in with support for this! I maintain jsx-pragmatic which is currently using Flow (and uses jsx in about the most dynamic way possible), tried to start converting it to TS today and ran into this. Definitely happy to play around with any beta builds and help test it out when there's something going here. 👍

[bcherny]

Related: Flow just announced AbstractComponent, which is necessary to correctly support a lot of newer React features, at least in Flow-land - https://medium.com/flow-type/supporting-react-forwardref-and-beyond-f8dd88f35544

[mterrel]

Definitely would love to see this fixed for our project. 😺

[dontsave]

I've been looking for a way to strongly type React children in TS for years now. Is this something that's on the roadmap at all?

[weswigham]

Well, there's a PR open with an implementation - #29818. It's really just a question of weather we think it's likely to be abused to too easily create programs which take an exponential time to check. (You'd think not, since it's no worse than nesting generic call expressions, and yet people don't nest call expressions 15 levels deep but do nest JSX tags that deep).

[dontsave]

thanks @weswigham. Seems tricky. I routinely encounter very deeply nested JSX tags in real world code, so I could see real issues there. Would it make sense to be an opt-in feature?

[dead-claudia]

@weswigham

people don't nest call expressions 15 levels deep but do nest JSX tags that deep

Yes, people do, believe it or not. You see it quite often in tree-building DSLs like hyperscript where the DSL is based in JS rather than transpiling to it. They don't usually hit 15, but 5-10 is pretty common and I've seen people get rather close to 15. In my experience, people nest hyperscript a little more deeply than they do with JSX because of the added conciseness of using h("div.foo", h("div.bar", ...)) instead of <div className="foo"><div className="bar">...</div></div> with React or <div class="foo"><div class="bar">...</div></div> with most other libraries. You'll also have similar concerns with Vue's render function which, although it lacks the dynamic selector support, is otherwise pretty similar to normal hyperscript DSLs. Other tree-building DSLs have similar constraints like babel-types, where in my experience, the typical call nesting depth is about 1.5x as deep as it is with typical JSX (1-3 in AST node building is like 1-2 in JSX, and 10-15 is like 5-10 in JSX).

[777PolarFox777]

As far as I understand, this issue was in TS 2.8 milestone. So now TS 3.6 is coming, but this wasn't fixed yet? I also can't find this in Roadmap, is there any work on this problem? Cause I'm still unable to return string from Functional Component and wrapping everything in React.Fragment gets annoying.

[felipeleusin]

I'm not sure the issue your describing is correct. I think the typing is wrong on this.

If you check https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/react/index.d.ts#L496 you can see that it accepts ReactElement | null but it should probably be ReactElement | Array<ReactElement> | string | number | null that are the supported elements to return from a functional component. Not sure why this is happening there, might be worth investigate

[danielkcz]

@felipeleusin @777PolarFox777 I already tried to a fix way back, but there was never a conclusion what to do about it...

https://github.com/DefinitelyTyped/DefinitelyTyped/pull/23422

[orta]

Also worth noting, that this issue has a PR https://github.com/microsoft/TypeScript/pull/29818 which is requesting folks test it on their codebases to get a sense of the performance implications - you can help that PR get merged by giving it a test run and posting the results in the PR 👍

[avonwyss]

I'm hitting the same roadblock: in a React web app, I also created code in separate files to emit plain XML documents with JSX and used the @jsx pragma. That works fine for the code emit, but the syntax checker is completely off and throws errors, for instance TS2345.

For now I can work around with an any cast like the following, but that's far from ideal.

/** @jsx Xml.createElement */

Xml.renderDocument(
    <worksheet xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main">
    </worksheet> as any)

In addition to that, I had to add [xml: string]: any; to the JSX.IntrinsicElements interface, I could not find any other workaround for the IntrinsicElements errors.

[weswigham]

@avonwyss in your case, you should just define Xml.JSX.Element - we support looking up custom nonglobal jsx namespaces for custom pragmas. This issue is mostly concerned with the ability for each tag, within a single jsx context, to have its own kind of return type (so you can distinguish different kinds of child tags).

[gfmio]

First of all, thank you for all the awesome work on TypeScript. I love working with it and I hope it continues to improve.

I've looked at this problem again very recently as well for my library typed-jsx. It provides basic tools for working with JSX and creating JSX toolkits. https://github.com/gfmio/typed-jsx

The two JSX factory functions jsx and data that this package exposes are strongly typed, i.e. if I invoke the JSX factory directly, the type checker can infer the generic type parameters of the results automatically and thus guarantee type safety.

In contrast, when using JSX syntax, all of this information is lost - and unnecessarily so.

One of the examples I use in the code is to use the data jsx factory function to construct an object hierarchy representing file objects.

/* @jsx data */

import { data } from 'typed-jsx';

enum FileType {
  File = 'file',
  Directory = 'directory',
  SymbolicLink = 'symlink',
}

interface Directory {
  type: FileType.Directory;
  name: string;
  children: Array<File | Directory | SymbolicLink>;
}
const Directory = (props: Omit<Directory, 'type'>) => ({ type: FileType.Directory, ...props });

interface File {
  type: FileType.File;
  name: string;
  data: Buffer | string;
}
const File = (props: Omit<File, 'type'>) => ({ type: FileType.File, ...props });

interface SymbolicLink {
  type: FileType.SymbolicLink;
  name: string;
  target: string;
}
const SymbolicLink = (props: Omit<SymbolicLink, 'type'>) => ({ type: FileType.SymbolicLink, ...props });

const files = (
  <Directory name="root">
    <Directory name="sub">
      <File name="file1" data="..." />
    </Directory>
    <Directory name="sub2">
      <File name="file2" data="..." />
      <SymbolicLink name="file3" target="../sub/file1" />
    </Directory>
  </Directory>
);

console.log(JSON.stringify(files, undefined, 2));
// Will print:
//
// {
//   "type": "directory",
//   "name": "root",
//   "children": [
//     {
//       "type": "directory",
//       "name": "sub",
//       "children": [
//         {
//           "type": "file",
//           "name": "file1",
//           "data": "..."
//         }
//       ]
//     },
//     {
//       "type": "directory",
//       "name": "sub2",
//       "children": [
//         {
//           "type": "file",
//           "name": "file2",
//           "data": "..."
//         },
//         {
//           "type": "symlink",
//           "name": "file3",
//           "target": "../sub/file1"
//         }
//       ]
//     }
//   ]
// }

Writing this using JSX feels natural and it's terse, especially compared to actually writing out the function definitions using normal syntax.

In my current set-up, the type data.JSX.JSXElement is any, so I cannot perform adequate type checking of the children of Directory, even though this would be perfectly possible when invoking the equivalent data() calls directly.

Another, more advanced use case for this would be using JSX for describing services registrations for an IOC framework, where nested services define error boundaries, component lifecycle events correspond to service lifecycle events and components can define configurable service registration patterns.

JSX is nothing but syntactic sugar for invoking a factory function. Hence, I think, it can and should be treated that way and not like a special black box, as it currently is.