Skip to content
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

fix(react): unions in prop types are not resolved #844

Merged
merged 1 commit into from
Sep 22, 2021

Conversation

dewey92
Copy link

@dewey92 dewey92 commented Sep 22, 2021

Motivation

Suppose I have:

type GridProps =
  | { container?: false }
  | { container: true; spacing: number }
declare const Grid: React.FC<GridProps & { className?: string }>

// ❌ should not compile
const testGrid = <Grid container={false} spacing={8} />

/**
Type '{ container: false; spacing: number; }' is not assignable to type 'IntrinsicAttributes & PropsWithChildren<GridProps & { className?: string | undefined; }>'.
     Property 'spacing' does not exist on type 'IntrinsicAttributes & { container?: false | undefined; } & { className?: string | undefined; } & { children?: ReactNode; }'.
*/

// ✅ compiles
const testGrid = <Grid container={true} spacing={8} />

The idea is to have an extra guard where you can only pass spacing as a prop when container is true. But unfortunately the current typing doesn't handle this case very well yet: passing this component to linaria's styled doesn't even compile.

const LinariaGrid = styled(Grid)``

and yields this rather long error message:

No overload matches this call.
     Overload 1 of 3, '(tag: (FunctionComponent<{ container?: false | undefined; } & { className?: string | undefined; }> & FC<GridProps & { className?: string | undefined; }>) | (FunctionComponent<...> & FC<...>)): ComponentStyledTag<...>', gave the following error.
       Argument of type 'FC<GridProps & { className?: string | undefined; }>' is not assignable to parameter of type '(FunctionComponent<{ container?: false | undefined; } & { className?: string | undefined; }> & FC<GridProps & { className?: string | undefined; }>) | (FunctionComponent<...> & FC<...>)'.
         Type 'FunctionComponent<GridProps & { className?: string | undefined; }>' is not assignable to type 'FunctionComponent<{ container?: false | undefined; } & { className?: string | undefined; }> & FC<GridProps & { className?: string | undefined; }>'.
           Type 'FunctionComponent<GridProps & { className?: string | undefined; }>' is not assignable to type 'FunctionComponent<{ container?: false | undefined; } & { className?: string | undefined; }>'.
             Types of property 'propTypes' are incompatible.
               Type 'WeakValidationMap<GridProps & { className?: string | undefined; }> | undefined' is not assignable to type 'WeakValidationMap<{ container?: false | undefined; } & { className?: string | undefined; }> | undefined'.
                 Type 'WeakValidationMap<{ container: true; spacing: number; } & { className?: string | undefined; }>' is not assignable to type 'WeakValidationMap<{ container?: false | undefined; } & { className?: string | undefined; }>'.
                   Types of property 'container' are incompatible.
                     Type 'Validator<true> | undefined' is not assignable to type 'Validator<false | null | undefined> | undefined'.
                       Type 'Validator<true>' is not assignable to type 'Validator<false | null | undefined>'.
                         Type 'true' is not assignable to type 'false | null | undefined'.
     Overload 2 of 3, '(tag: ComponentType<{ container?: false | undefined; } & { className?: string | undefined; }> | ComponentType<{ container: true; spacing: number; } & { className?: string | undefined; }>): ComponentStyledTag<...>', gave the following error.
       Argument of type 'FC<GridProps & { className?: string | undefined; }>' is not assignable to parameter of type 'ComponentType<{ container?: false | undefined; } & { className?: string | undefined; }> | ComponentType<{ container: true; spacing: number; } & { className?: string | undefined; }>'.
         Type 'FunctionComponent<GridProps & { className?: string | undefined; }>' is not assignable to type 'FunctionComponent<{ container?: false | undefined; } & { className?: string | undefined; }>'.
           Types of property 'propTypes' are incompatible.
             Type 'WeakValidationMap<GridProps & { className?: string | undefined; }> | undefined' is not assignable to type 'WeakValidationMap<{ container?: false | undefined; } & { className?: string | undefined; }> | undefined'.
               Type 'WeakValidationMap<{ container: true; spacing: number; } & { className?: string | undefined; }>' is not assignable to type 'WeakValidationMap<{ container?: false | undefined; } & { className?: string | undefined; }>'.
                 Types of property 'container' are incompatible.
                   Type 'Validator<true> | undefined' is not assignable to type 'Validator<false | null | undefined> | undefined'.
                     Type 'Validator<true>' is not assignable to type 'Validator<false | null | undefined>'.
                       Type 'true' is not assignable to type 'false | null | undefined'.
     Overload 3 of 3, '(tag: keyof IntrinsicElements): HtmlStyledTag<keyof IntrinsicElements>', gave the following error.
       Argument of type 'FC<GridProps & { className?: string | undefined; }>' is not assignable to parameter of type 'keyof IntrinsicElements'.
         Type 'FunctionComponent<GridProps & { className?: string | undefined; }>' is not assignable to type '"view"'.

This is because the union type is distributed when acted on a conditional type, as described here.

Summary

The fix is quite simple: we add an extra bracket to every position in which the "prop" type variable is mentioned e.g [T] extends React.ComponentType<any> ? ... : ... to make the type non-distributive.

@Anber
Copy link
Collaborator

Anber commented Sep 22, 2021

Thank you so much!
I also faced that problem after upgrade to ts 4.4, but didn't have enough time to investigate it.

@Anber Anber merged commit d0f9c78 into callstack:2.0.x Sep 22, 2021
@dewey92
Copy link
Author

dewey92 commented Sep 22, 2021

I can create another PR for the master branch, or you can also do it by yourself :)

@Anber
Copy link
Collaborator

Anber commented Sep 22, 2021

I'll cherry-pick it. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants