[WIP] Stronger types for React DOM

[motiz88]

Provides stronger prop type definitions for React DOM's JSX intrinsics.

This will be an ongoing project for a little while as the scope is ambitious. I'm opening this WIP PR for early feedback on the overall direction as well as for help from fellow interested developers. Pull requests to this branch welcome!

Will eventually close #3773 and close #5865.

Includes #6728 (SyntheticPointerEvent definition).

Implementation checklist

• Global HTML attributes (except for events)
  • autoCapitalize (missed this in the initial pass)
• Global HTML events
• Global SVG attributes and events
• Element-specific attributes (and events, if that's a thing)
• Precise type for style prop (a whole other can of worms 😅)
• Disallow children and dangerouslySetInnerHTML on void element tags
• Accurate instance types (for refs and Event.currentTarget)
• More? Let's discuss in this PR

Useful references

• DOMProperty.js in react-dom
• react-dom event system implementation and docs
• The HTML living standard and MDN, naturally

Design choices / constraints

(I am open to discussing all of these!)

Leave unknown props typed as any

There is no way we can enumerate all acceptable/standard props of DOM intrinsics. If we ever got a Flow feature allowing regexes for keys, we could express open-ended namespaces like data-* attributes, custom attributes etc more precisely, and then maybe revisit this - with the goal of detecting more classes of errors, such as typos in prop names, or usage of attributes on elements that don't support them.

For now the typing philosophy here is: be reasonably strict with the types of known attributes, but lenient otherwise. This means we can add some value with these types, but not quite match the DX of using composite components... yet.

• Open question: Should these be any or mixed? The latter would seem to be more useful in scenarios other than passing attributes into elements (say, defining a custom component type that takes React.ElementProps<button>)

Type numeric attributes as number

React will not warn if you pass a string (be it "1" or "foo") into what's meant to be a numeric attribute, e.g. tabIndex. However, I'm leaning towards the stricter tabIndex?: number in such cases. It's a judgement call, though, and an argument can be made for number | string. I've created the ReactDOM$Number type alias as a reminder to come back to this after some discussion.

Impose some case-sensitivity on enumerated attributes

In an ideal world we'd have a Flow builtin, perhaps called $CaseInsensitive, that makes the following possible:

const valid: $CaseInsensitive<"foo" | "bar"> = "Foo"; // OK
const invalid: $CaseInsensitive<"foo" | "bar"> = "baz"; // Error

(EDIT: See separate issue about $CaseInsensitive - #6743)

With such a hypothetical builtin (and I'm not the right person to build it - as I'm not fluent in OCaml by any stretch), we could model enumerated attributes correctly, as case-insensitive enums.

Here, rather than reverting to string for such attributes (or manually adding all letter case permutations of values), I've arbitrarily chosen to support what I think are common/natural-enough spellings.

• For enumerated values made of single words (true, false) - support only the all-lowercase version.
• For enumerated values representing acronyms (ltr, rtl) - support all-lowercase alongside all-uppercase (LTR, RTL).
• For enumerated values representing multiple joined words (contentEditable) - support camel-case alongside all-lowercase (contenteditable).
• For enumerated values in kebab-case (family-name, street-address) - only support the lowercase version.

I am totally hoping that a Flow internals expert can come along and just give us $CaseInsensitive 😄 In the meantime I've introduced the $TODO$CaseInsensitive type as a placeholder.

Forbid the value "" in all kinds of boolean/enumerated props

See also #6727 (comment), facebook/react#13400, facebook/react#13404.

There are a few classes of DOM attributes that are "boolean", "booleanish" (a term from the React source) or "enumerated"; in HTML, these generally admit "" as a valid value meaning "true". However, for some props, React DOM processes "" as a falsy JS value, omitting the attribute. This inconsistency, along with the fact that there is always a less-ambiguous way to specify "true" (foobar={true} or foobar="foobar") or an equivalent, is why I chose to model these props with union types that do not include "" - reducing the chance of users stumbling across surprising behaviour.

[motiz88]

UPDATE: This is now described as a design choice in the main comment: #6727 (comment)

---

In light of the issue outlined in facebook/react#13400, I'm currently thinking of removing "" as an allowed value from all kinds of enumerated attributes (ReactDOM$Boolean, ReactDOM$BooleanishString, and others like translate?: $TODO$CaseInsensitive<"" | "yes" | "no">), as there are simply too many subtly different behaviours to keep track of.

• In a DOM boolean prop - modeled by ReactDOM$Boolean - React treats "" as falsy, while HTML specifies it as one of the allowed truthy values.
• In DOM "booleanish string" props - ReactDOM$BooleanishString - and other enums where "" is allowed, React treats it as truthy (as does HTML).

As with typing numeric properties as number, disallowing "" is an "editorial" decision to be stricter in order to reduce ambiguity and the potential for bugs. I'd be happier to do this if React were to add a corresponding warning or documentation note discouraging the use of "" (or string values in general) in DOM boolean props.

[mrkev]

Oh, nice. Some comments:

• It might be helpful to do this work throughout multiple PRs, and keep track of progress in a "master issue?

• In an ideal world we'd have a Flow builtin, perhaps called $CaseInsensitive. Interesting idea, and can definitely see cases such as this in which it would be useful. Making an issue to track the addition said utility and liking it back here for context would be helpful to keep track of it.

• I'm a little worried about making stuff more restrictive, since it could introduce errors to people's pre-existing builds. Up for discussion.

• Digging the pointer event's definitions 👏!

[motiz88]

@mrkev:

It might be helpful to do this work throughout multiple PRs, and keep track of progress in a "master issue?

Yes in principle, I'm just trying to get to a point where I can see the overall "shape" of this work & see what the shippable milestones are along the way. Happy to get your thoughts on that. Also I didn't know what to expect in terms of a timeline for review/release etc (I'm glad to have your comments this early!), and as you see I'm still discovering some issues as I dive into specs and React's implementation thereof.

Things like modelling style can definitely be broken out, but in my mind the ideal is to roll all of this up into one coherent release of Flow - "the one where we got typechecking for React DOM" (and had to adjust a little bit 😄). One clear transition might be easier to document and educate users about.

[motiz88]

This is still in progress, just had to take a break and focus on other things recently.

There is an angle here that I think ties into the React team's longer-term plans for React DOM (facebook/react#13525). If ultimately we can model both today's React DOM and the future "React Fire" (which will include breaking API changes, e.g. potentially className → class and others), and the types are accurate and strict enough, then Flow can play a useful role in helping the ecosystem migrate safely, especially for use cases not covered by non-type-aware codemod tools. The ideal starting point, well in advance of when React starts warning/deprecating APIs or committing to specific plans, would IMHO be a good typechecking experience for React DOM code written today.

[goodmind]

@motiz88 are you still working on this PR?

[villesau]

@jbrown215 would there be parts in this that could be used from this pull request?? This is needed for improved Flowgen React support.

[jbrown215]

@villesau: Which parts do you need? I expect that @motiz88 does not have time to continue his work on this, but I'm happy to review other PRs to the libdefs that you need.

[villesau]

I need as much as possible 😅This is the beef: joarwilk/flowgen#76 (comment)

I'll try to pick up the parts that are needed the most.

[goodmind]

Yeah, actually part that is needed is not implemented here
Element-specific attributes (and events, if that's a thing)

Also style property can be typed with this https://github.com/frenic/csstype

[FezVrasta]

Is there any chance to get this revived?

[gkz]

Subsumed by #7569