flow icon indicating copy to clipboard operation
flow copied to clipboard

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

Open aldendaniels opened this issue 7 years ago • 6 comments

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?

aldendaniels avatar Apr 19 '17 21:04 aldendaniels

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>;
    }
}

mwalkerwells avatar Apr 22 '17 00:04 mwalkerwells

@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.

aldendaniels avatar Apr 24 '17 15:04 aldendaniels

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 avatar Jan 30 '18 17:01 Peeja

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.

Peeja avatar Jan 30 '18 17:01 Peeja

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

motiz88 avatar Aug 06 '18 16:08 motiz88

@motiz88 thank you for taking this on!

cprodescu avatar Aug 06 '18 19:08 cprodescu