types/react-dom/index.d.ts

to authors (@pspeter3 @vsiao @MartynasZilinskas AssureSign (account can't be detected) Microsoft (account can't be detected)). Could you review this PR?
👍 or 👎?

Checklist

• pass the Travis CI test?

types/react/index.d.ts

to authors (@pspeter3 @vsiao @johnnyreilly @bbenezech @pzavolinsky @digiguru @ericanderson @morcerf @tkrotoff @DovydasNavickas @onigoetz @richseviora AssureSign (account can't be detected) Microsoft (account can't be detected)). Could you review this PR?
👍 or 👎?

Checklist

• pass the Travis CI test?

Uhmm, I guess something is clearly wrong. Checking it out.

@cynecx Please fix the failures indicated in the Travis CI log.

It seems that this patch doesn't affect any of the occurring errors as I can reproduce them even without this patch. Would be great if someone could review this. Thank You.

Rebased the feature branch.

It seems that there is an issue in react-router/test/InheritingRoute.tsx. The problem here is that the return type of the render function (JSX.Element | JSX.Element[]) is not compatible with the paramter type of the cloneElement function signature as it only accepts JSX.Element.

I am not sure how to fix this issue. We can either assume that the render function in the class Route from react-router always returns a JSX.Element or we can modify the test.

We could modify the Route class like the following:

export class Route<T extends RouteProps = RouteProps> extends React.Component<T> {
  render(): JSX.Element | null | false;
}

Should this go in a new PR?

@cynecx I'm thinking it's probably best to update the broken tests (react-router and other types as necessary) and then flag their contributors about the change for their approval.

Rebased on master. I am working on fixing the tests now.

Seems good now. There is just one error which I am certain that this PR didn't affect:

Error in redux-form v6
Command failed: node /home/travis/build/DefinitelyTyped/DefinitelyTyped/node_modules/dtslint/bin/index.js --noLint
Unexpected compiler option noImplicitReturns

Flagging some users for approval: @prakarshpandey, @Andy-MS, @mtpc, @tkrotoff, @MartynasZilinskas

Any update on this?

@circlingthesun Well, I am still currently waiting for approval :)

Hi @cynecx, tests should be fixed if you merge in the latest master.

@Andy-MS Okay. I am not sure how this PR caused that last one but I'll fix it :)

Thanks all!

It seems it breaks a simple case:

import * as React from "react";
import * as ReactDOM from "react-dom";

class Main extends React.Component<{}, {}> {}

ReactDOM.render(<Main />, document.getElementById("container")); // <- error here

error TS2345: Argument of type 'Element' is not assignable to parameter of type 'ReactElement<any>[]'.
  Property 'length' is missing in type 'Element'.

@plantain-00 I am not able to reproduce this. Your simple case compiles just fine here. What version of typescript are you currently using?

@cynecx

"@types/react": "16.0.10",
"@types/react-dom": "16.0.1",
"typescript": "2.5.3",

wait for a while.
update: it's npm's problem, there is a node_modules/@types/react/* in @types/react-dom

@plantain-00 could you put together a minimal repro repo to share please? That will help things move quicker

Based on @plantain-00's observation, I managed to work around this by adding the following to my package.json (I'm using yarn).

"resolutions": {
    "@types/react": "16.0.10",
    "@types/react-dom": "16.0.1",
}

it's OK now, there is a node_modules/@types/react-dom/node_modules/@types/react/* in node_modules/@types/react-dom, reinstalling them works

This does not work:

  render() {
    return this.props.children;
  }

Question: why do we have ReactNode and JSX.Element? Could they be merged?

@tkrotoff Since when we can return undefined in render()? You can only return null in render method if I remember correctly. Unless it was changed in newest React v16 that if children are absent it will return null instead of undefined.

@MartynasZilinskas You're right, see https://codepen.io/tkrotoff/pen/yzKKdB

Still there is a problem:

  render() {
    const { children } = this.props;
    return children !== undefined ? children : null;
  }

Types of property 'render' are incompatible.
  Type '() => string | number | boolean | {} | ReactElement<any> | (string | number | boolean | any[] | R...' is not assignable to type '() => string | number | false | Element | Element[] | null'.
    Type 'string | number | boolean | {} | ReactElement<any> | (string | number | boolean | any[] | ReactEl...' is not assignable to type 'string | number | false | Element | Element[] | null'.
      Type 'true' is not assignable to type 'string | number | false | Element | Element[] | null'.

Has the above fix been added for stateless components? I'm getting an error when attempting to implement one that returns an array of elements

why was the generic version of findDOMNode removed?

export function findDOMNode<E extends Element>(instance: ReactInstance): E;

(tagging @cynecx)

@patsissons it was removed because of a lint failure. Does it suffice to convert usages like so?

before:

ReactDOM.findDOMNode<HTMLInputElement>(inst)

after:

<HTMLInputElement>ReactDOM.findDOMNode(inst)

yea i am doing (basically) this as a temporary patch. what is the type safe way of finding a dom node of a specific type (i.e., my use case is that i am finding a dom node that will then be focused, so i need the type to be derived from HTMLElement)? I don't particularly enjoy using direct cast (or the as operator in my case, since it is in a tsx file).

I guess the argument is that the previous typing was no more type safe than a direct cast, in which case I understand why the change was made.

@patsissons, you will NOT get a type-safe result from a function ReactDOM.findDOMNode, because TypeScript's type system exists only during compile time and you are calling a function that was defined in JS world (react-dom library).

What you CAN do is have a type-gate for checking if an element.focus != null && typeof element.focus === "function". This way you can create and use your own Focusable interface for that and have a type-safety this way.

@patsissons I think the typecast is equivalent to what you were doing before. As @DovydasNavickas mentioned there's no guarantee that findDOMNode will return the exact subtype of Element that you expect.

I have to imagine that the equivalence of findDOMNode<T>(elem) would be identical to findDOMNode(elem) as T. I was using the generic call with a semantic guarantee that it would return a sane dom node. But I now see that syntactically there ought to be no difference since the cast is just happening through generic parameterization rather than direct casting. Since i was already sanitizing the dom node anyways, it really doesn't matter other than requiring a direct cast which i try to avoid but in this case it is literally unavoidable (unless i create a type guard function to recast back to HTMLElement).

@cynecx I think you forgot about functional components. The following code doesn't work

const Test: React.StatelessComponent<any> = () => [
  <div />,
  <div />,
]

and the error

[ts]
Type '() => Element[]' is not assignable to type 'StatelessComponent'.
Type 'Element[]' is not assignable to type 'ReactElement | null'.
Type 'Element[]' is not assignable to type 'ReactElement'.
Property 'type' is missing in type 'Element[]'.

@niba Seems so. Looks like a trivial change to me. I can open a PR later.

@cynecx I have been trying to add this on my own but it doesn't look like a trivial change. Simple changing the return type of StatalessComponent function breaks JSX
from (props: P & { children?: ReactNode }, context?: any): ReactElement<any> | null;
to (props: P & { children?: ReactNode }, context?: any): ReactElement<any>[] | ReactElement<any>| null;

with this change, the below definition of functional component started working

const StatelessComponent: React.SFC<SCProps> = ({ foo }: SCProps) => {
  return [
    <div>a</div>,
    <div> aa</div>
  ];
};

but now it's impossible to call it from JSX. <StatelessComponent />; produces

[ts]
JSX element type 'ReactElement | ReactElement[] | null' is not a constructor function for JSX elements.
Type 'null' is not assignable to type 'ElementClass'.
[ts] JSX element class does not support attributes because it does not have a 'props' property.

I don't see any constructor function in ReactElement and I don't know how JSX is processed by typescript. Do you have any idea what is wrong?

@niba You are right, this doesn't look trivial. I am also not sure what's wrong here (I am not too familiar with this code) but I will try investigating this.

I don't have much time to look into this, but there's some useful background in microsoft/TypeScript#5478 about how SFC+TSX works. cc @RyanCavanaugh

I get an error when I try to return an array that contains a null element from render(). Simplified example:

class ReturnArrayWithNullElement extends React.Component<Props, {}> {

    ...

    render() {
        return [
            <div key={1}>Element 1</div>,
            <div key={2}>Element 2</div>,
            this.props.displayThird ? <div key={3}>Element 3</div> : null
        ]
    }
}

Maybe this isn't an idiomatic way of conditionally rendering an element inside of an array (still becoming acquainted with Fiber), but React deals with this how I would expect. TypeScript throws the following error:

error TS2415: Class 'ReturnArrayWithNullElement' incorrectly extends base class 'Component<Props, {}>'.
  Types of property 'render' are incompatible.
    Type '() => (Element | null)[]' is not assignable to type '() => string | number | false | Element | Element[] | ReactPortal | null'.
      Type '(Element | null)[]' is not assignable to type 'string | number | false | Element | Element[] | ReactPortal | null'.
        Type '(Element | null)[]' is not assignable to type 'ReactPortal'.

@notitatall the return type of render needs to be merged with the ReactNode type. For the time being you can probably cast to an appropriate type to unblock yourself.