Definitelytyped: React typings allow false for class components, but not for stateless

[magnushiie]

• [x] I tried using the @types/xxxx package and had problems.
• [x] I tried using the latest stable version of tsc. https://www.npmjs.com/package/typescript
• [x] I have a question that is inappropriate for StackOverflow. (Please ask any appropriate questions there).
• [x] [Mention](https://github.com/blog/821-mention-somebody-they-re-notified) the authors (see Definitions by: in index.d.ts) so they can respond.
  
  
  
  • Authors: @gaspard @vbfox
  
  

As mentioned in React documentation, a stateless component can return false for empty content. Currently StatelessComponent is defined as:

interface StatelessComponent<P = {}> {
        (props: P & { children?: ReactNode }, context?: any): ReactElement<any> | null;

whereas Component has

class Component<P, S> {
        constructor(props?: P, context?: any);
        // ...snip...
        render(): JSX.Element | null | false;

Shouldn't these types be the same (including eliminating the ReactElement vs JSX.Element difference)? And to avoid future such issues (#14064, this one) it should probably be a named type, e.g. RenderResult.

A search for | null also comes up with ComponentSpec.render which probably should also have the same result type.

[magnushiie]

Note: React 16 will allow arrays and strings and arrays of strings also. So at one point it would be similar to ReactNode.

Also, if unifying with ReactNode, the recursive definition can be done as described by @ahejlsberg in https://github.com/Microsoft/TypeScript/issues/3496#issuecomment-128553540

[vbfox]

I mentioned it here https://github.com/DefinitelyTyped/DefinitelyTyped/pull/17021#issuecomment-307027890 but forgot to follow or PR.

[gaspard]

@vbox Using a type for this is a good idea. Please also add a test for any new elements in this return type (like false). :-)

[adrienharnay]

Hey, is anyone working on this? If not I could help finish this if needed :)

[mgol]

Currently, the class Component type specifies its render method type as:

render(): ReactNode;

Shouldn't FunctionComponent type be changed from:

(props: P & { children?: ReactNode }, context?: any): ReactElement<any> | null;

to:

(props: P & { children?: ReactNode }, context?: any): ReactNode;

as well? As far as I understand both return types should be identical. Current types make it hard to refactor a class component as a function one as then suddently the return types change.

[Hotell]

@Kovensky did this PR AFAIR , any reasons why FC returns ReactElement | null instead of ReactNode

I guess it might be related that ReactNode contains {} and undefined which makes compoennt returns type checking hard https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/react/index.d.ts#L171

[Jessidhia]

That is because the compiler is still dumb in this respect and thinks function components have to return JSX.Element | null.

function Test() { return null }
function Test2() { return false }

const jsx = (
  <>
    <Test />
    <Test2
      // type error
      // "JSX element type 'boolean' is not a constructor function for JSX elements."
      // (looks like the error message the bug causes is itself buggy???)
    />
  </>
)

This is unrelated to the type of FunctionComponent, it's "knowledge" hardcoded into the compiler, and correcting the type in FunctionComponent would actually make it impossible to use things typed or union-ed with FunctionComponent as JSX tags.

This also extends to returning string, number, or ReadonlyArray<React.ReactElement<any> | string | number | boolean | null | undefined>, all valid return types in React 16.

(Note that React.ReactNode is not a valid return type, btw)

---

The _other_ problem I wanted to fix is that class components' render()s are currently allowed to return undefined but that's a runtime error. They're also allowed to return {}, functions, and even Symbol(); could be a fun game to try to figure out if they crash at runtime or if React throws a proper warning. They're also allowed to not specify a render() at all and that's also a runtime error; class Component should be made abstract.

To fix the root cause of that problem I need that big 600+ files PR that takes too long to build for Travis (50min and it's not even 20% done). I'm just leaning more and more towards making a hard breaking change in the React types, rewriting and throwing everything legacy away. I wonder if typesVersions would provide a way to do that without breaking DefinitelyTyped itself -- dependent types could be updated on demand, on likely much smaller PRs. Right now the react types are just getting more and more ossificated.

[adrienharnay]

Hey, any news on this?

[eps1lon]

This can be fixed once Microsoft/TypeScript#21699 is resolved.

[TomasHubelbauer]

This seems to be the reason for why React.FunctionComponent (and React.SFC) cannot return ReactNode (because it contains false and null and undefined), but could they return ReactChild instead of ReactElement?

Right now this is an error:

const test: React.FunctionComponent<{}> = props => 'test';

I would like to propose that FunctionComponent returns ReachChild instead, but I am not sure whether that's the way to go. Can anyone advise?

[AviVahl]

Almost opened another duplicate... () => 'Hello World' should be a valid FunctionComponent. :/