facebook / flow

Adds static typing to JavaScript to improve developer productivity and code quality.
https://flow.org/
MIT License
22.08k stars 1.85k forks source link

React DOM (e.g <div />) not type checked #3773

Open aldendaniels opened 7 years ago

aldendaniels commented 7 years ago

Flow allows this without complaint:

import React from 'react';

class Test extends React.Component {
  render() {
    return <div style={1} bogus="allowed">Test</div>;
  }
}

Try

Should we add a disclaimer to this effect to the Flow + React until this level of type-safety is supported?

mwalkerwells commented 7 years ago

You mean add a disclaimer that the default return type of render is ?React$Element<any>? https://github.com/facebook/flow/blob/master/lib/react.js#L32

Not sure how useful that would be. If you need more type safety, you can easily add it:

import React from 'react';

class Test extends React.Component {
    render(): React$Element<{ style: number, bogus: string}> {
        return <div style={1} bogus="allowed">Test</div>;
    }
}
aldendaniels commented 7 years ago

@mwalkerwells Yeah, that's what I mean - there are not type defs for the React primitives and it's not clear how to provide any. By contrast, Typescript documents how to provide your own types for JSX intrinsics.

Regardless, I'd (incorrectly) expected types for these - and thought a disclaimer to this effect might be helpful to others. Feel free to close if you feel otherwise. Otherwise I'm happy to submit a PR.

I expect that the simplest way to get strongly typed primitives is to write wrapping components for the various DOM primitives that are strongly typed - in the spirit of react-primitives, but sticking to DOM spec - this is something I've been looking into for use in my own projects.

Peeja commented 6 years ago

We would find it extremely useful to type check props on intrinsics.

That appears to be what the issue title is about, but not what @mwalkerwells's clarification points to, unless I'm misunderstanding. As a simpler example, I'd expect this to fail:

<div style={1} bogus="allowed">Test</div>;

We sometimes spread props into intrinsic components, and it would be valuable to know when we've spread something that includes props React doesn't expect there, or of the wrong type.

Peeja commented 6 years ago

Also, it would be extra nice to have, for instance, <button>'s onClick prop automatically typed as (SyntheticEvent<HTMLButtonElement>) => void. We've passed handlers expecting to be attached to the wrong kind of DOM element before, and this would catch that.

motiz88 commented 6 years ago

I'd like to claim this issue please 😃 Will start working on it later this week.

cprodescu commented 6 years ago

@motiz88 thank you for taking this on!