Skip to content

JSX generic types that are themselves props are being incorrectly inferred in TS@next #28394

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Jessidhia opened this issue Nov 7, 2018 · 3 comments
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@Jessidhia
Copy link

Jessidhia commented Nov 7, 2018

TypeScript Version: 3.2.0-dev.20181107

Search Terms: I was told to make an issue 🤷‍♀️

Code

This is something I encountered while working on DefinitelyTyped/DefinitelyTyped#30331; there are at least two instances of generics that are incorrectly inferred on TS@next.

It seems the compiler is overly eager to add things to the generic parameter that are not actually necessary, with incomplete types (they're just {}) and thus they end up causing errors instead. For example,

                            <FieldArray
                                name="field9"
                                component={ MyArrayField }
                            />

is inferring FieldArray as FieldArray<{ name: {}; component: {} }> even though name and component are already present in FieldArray<{}>; this in turn causes MyArrayField to be expected to receive { name: {}; component: {} } as props, while it only receives {}.

A similar issue happened with <Button {...props}/> in the reactstrap test, see comment chain. The spread of props caused Button to basically infer its own props as being & Record<keyof props, {}>, which more obviously broke the type of children.

@Jessidhia Jessidhia changed the title JSX generics are being incorrectly inferred in TS@next JSX generic types that are themselves props are being incorrectly inferred in TS@next Nov 7, 2018
@Jessidhia
Copy link
Author

DefinitelyTyped/DefinitelyTyped#30331 (comment)

It seems that the condition for it to happen is that the generic parameter needs to itself be used as part of the props; not as the value of some of the props, but as props themselves.

class FieldArray<P = {}> extends Component<(...) & Partial<P>>
export type ButtonProps<T = {}> = (...) & T

@weswigham weswigham added Bug A bug in TypeScript Needs Investigation This issue needs a team member to investigate its status. labels Nov 7, 2018
@weswigham weswigham self-assigned this Nov 7, 2018
@weswigham weswigham added this to the TypeScript 3.2 milestone Nov 7, 2018
@weswigham
Copy link
Member

weswigham commented Nov 8, 2018

Whew, so this turned out to be an effect from an enhancement to inference we made (#28006). Specifically, prior to #28006 inference would fail at the reported locations, fall back to {}, then pass all checks. Now with #28006 in, we actually manage to create an inference for the Readonly<ButtonProps<T>> thing (where ButtonProps<T> is an intersection under the alias), except the way we do this is with a reverse mapped type (an internal construct for mapping a type backwards through a mapped type). This is usually OK, except in this case, to get the type of the children property on that reverse mapped type, we need to infer from ReactNode to ButtonProps<T>[P], searching for a T[P] as the inference goal. ButtonProps<T>[P] is really (ReactHTMLProps & { ... stuff ...} & T)[P] - so by our normal simplification rules (which we started to respect in inference with #27953), is equivalent to ReactHTMLProps[P] & {... stuff ...}[P] & T[P] (wherein T[P] is trivially found) - EXCEPT, since P is still generic, we don't simplify it (this way it only simplifies upon instantiation to make relationship checking work out more easily), fail to make an inference for the property, and fall back to a no inference result of {} - then {} gets intersected with ReactNode (from another intersection member that also defines the children without relying on the generic parameter capturing them), and you get the awful no good behavior described (that null & {} somehow becomes never might also be a bug, though!).

Quite simply, we want to force the distribution during reverse-mapped-property inference, since it may expose more of the type variables we're looking for directly to the inference engine. It is safe to do so, since despite the P type being generic, during inference we're attempting to match some T[P] where we treat T[P] as a potential type parameter in its own right - the P template is at this point shorthand for "any property", and safe to treat as though it were some specific property name, rather than an open ended type parameter (we're not going to instantiate it later - since this is inference, we're just trying to match it up to some other type as a pattern).

@weswigham weswigham added Fixed A PR has been merged for this issue and removed Needs Investigation This issue needs a team member to investigate its status. labels Nov 8, 2018
@weswigham
Copy link
Member

I've opened #28421 to track the behavior of intersecting {} with unit types and the issues it causes.

Jessidhia added a commit to Jessidhia/DefinitelyTyped that referenced this issue Nov 9, 2018
Jessidhia added a commit to Jessidhia/DefinitelyTyped that referenced this issue Nov 9, 2018
Jessidhia added a commit to Jessidhia/DefinitelyTyped that referenced this issue Nov 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

2 participants