Skip to content

Mapped Types with Intersections in React Props Definition, results in type is not assignable error #27201

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
IllusionElements opened this issue Sep 18, 2018 · 13 comments
Labels
Needs More Info The issue still hasn't been fully clarified

Comments

@IllusionElements
Copy link

IllusionElements commented Sep 18, 2018

Search Terms:
Mapped Types with React Props
incorrect assignable reference
Code
the intent is for this to be a decorator

type Mutate<T extends string | number | symbol> = Readonly<
  { [K in T]: MutationFn }
>
const withMutation = <
  Key extends string,
  V extends OperationVariables,
  TData extends object = any
>(
  mutationProps: Omit<MutationProps<TData, V>, 'children'>
) => {
  return <P extends object>(
    Component: ComponentType<
      P &
        Mutate<Key> &
        Pick<MutationResult<TData>, 'loading' | 'data' | 'error'>
    >
  ) =>
    class WithMutation extends React.Component<P> {
      render() {
        const key = (mutationProps.mutation
          .definitions[0] as OperationDefinitionNode).name!.value as K
        return (
          <Mutation {...mutationProps}>
            {(mutateFn: MutationFn, result: MutationResult<TData>) => {
              const error = result.error! 

              return (
                <Component
                  loading={result.loading!}
                  error={error}
                  {...result.data}
                  {...this.props}
                  {...{ [key]: mutateFn }}
                />
              )
            }}
          </Mutation>
        )
      }
    }
}

// A *self-contained* demonstration of the problem follows...
// Test this by running `tsc` on the command-line, rather than through another build tool such as Gulp, Webpack, etc.

Expected behavior:
the props should be correct, for reference the mapped type behavior produces this I think

type Mutate<T extends string | number | symbol> = Readonly<
  { [K in T]: MutationFn }
>

Mutate<'submitForm'> === Readonly<{ submitForm: MutationFn }>

not actually sure why it's saying the type is not assignable since
type '((IntrinsicAttributes & IntrinsicClassAttributes<Component<P & Readonly<{ [K in Key]: MutationFn<any, OperationVariables>; }> & Pick<MutationResult, "data" | "loading" | "error">, any, any>> & Readonly<...> & Readonly<...>) | (IntrinsicAttributes & ... 2 more ... & { ...; }))["loading"]'

should evaluate to boolean.
Actual behavior:
image

Playground Link:

Related Issues:

@dmichon-msft
Copy link
Contributor

The line {...result.data} doesn't match the defined type of the component props. The latter expects data={result.data}.
The most strictly correct props type reflected by your current decorator is:

ComponentType<{ [PropKey in (keyof P | Key | 'loading' | 'error' | keyof MutationResult<TData>)]: PropKey extends Key ? MutationFn : PropKey extends keyof P ? P[PropKey] : PropKey extends keyof MutationResult<TData> ? MutationResult<TData>['data'][PropKey] : PropKey extends ('loading' | 'error') ? MutationResult<TData>[PropKey] : never; }

@IllusionElements
Copy link
Author

IllusionElements commented Sep 20, 2018

even with data={result.data}, the compiler still complains that loading={result.loading as boolean} is not assignable to type { loading: boolean } in the props

What i'm guessing is the compiler can't figure out what type '((IntrinsicAttributes & IntrinsicClassAttributes<Component<P & Record<Key, MutationFn<any, OperationVariables>> & Pick<MutationResult<TData>, "data"> & { results: { loading: boolean; error: ApolloError; }; }, any, any>> & Readonly<...> & Readonly<...>) | (IntrinsicAttributes & ... 3 more ... & { ...; }))["error"]'
should evaluate to

after the mapped type is removed, the errors disappear

@IllusionElements
Copy link
Author

IllusionElements commented Sep 20, 2018

Actually it appears the issue stems from the fact that the component props are being defined as a intersection of two readonly types.

link to example: Repo

@RyanCavanaugh RyanCavanaugh added the Needs More Info The issue still hasn't been fully clarified label Sep 21, 2018
@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Sep 21, 2018

Please reduce the repro further - thanks!

@IllusionElements
Copy link
Author

reduced here

@weswigham
Copy link
Member

weswigham commented Sep 21, 2018

It's worth nothing that the error is correct, if awful to diagnose (at least in your reduced example). In your simplified example:

/** @format */
declare type Index = string | number | symbol

declare type Mapped<Key extends Index, T> = { [K in Key]: T }

declare type MutationFn<T, U> = any

type I<P, TData, Key extends string> = Readonly<{
   children?: any;
  }> & Readonly<P & TData & {
  loading: boolean,
  error: Error
  } & Mapped<Key, MutationFn<any, any>>>

type Z<P, D, K extends string> = I<P, D, K>["loading"]

const loading: boolean = true

class Example<P, D, K extends string> {
  isLoading: Z<P, D, K> = loading
}


type Other<P, TData, Key extends string> = Readonly<{
   children?: any;
  } & P & TData & {
  loading: boolean,
  error: Error
} & Mapped<Key, MutationFn<any, any>>>


type Tesr<P, D, K extends string> = Other<P, D, K>["loading"]

class Example2<P, D, K extends string> {
  isLoading: Tesr<P, D, K> = loading
}

if I instantiated Example as Example<{loading: false}, {}, "">, the isLoading member would be of type false & boolean which is false & (true | false), which is false & true | false & false, which is never | false, which is simply false, to which a boolean should not be assignable to!

@IllusionElements
Copy link
Author

IllusionElements commented Sep 22, 2018

It's worth nothing that the error is correct, if awful to diagnose (at least in your reduced example). In your simplified example:

/** @format */
declare type Index = string | number | symbol

declare type Mapped<Key extends Index, T> = { [K in Key]: T }

declare type MutationFn<T, U> = any

type I<P, TData, Key extends string> = Readonly<{
   children?: any;
  }> & Readonly<P & TData & {
  loading: boolean,
  error: Error
  } & Mapped<Key, MutationFn<any, any>>>

type Z<P, D, K extends string> = I<P, D, K>["loading"]

const loading: boolean = true

class Example<P, D, K extends string> {
  isLoading: Z<P, D, K> = loading
}


type Other<P, TData, Key extends string> = Readonly<{
   children?: any;
  } & P & TData & {
  loading: boolean,
  error: Error
} & Mapped<Key, MutationFn<any, any>>>


type Tesr<P, D, K extends string> = Other<P, D, K>["loading"]

class Example2<P, D, K extends string> {
  isLoading: Tesr<P, D, K> = loading
}

if I instantiated Example as Example<{loading: false}, {}, "">, the isLoading member would be of type false & boolean which is false & (true | false), which is false & true | false & false, which is never | false, which is simply false, to which a boolean should not be assignable to!

That makes sense, but then why would it still error, if I mark it so that types P & D have any boolean properties excluded from the type? and also explicitly mark the mapped type that it returns something other than a boolean

updated repo here

wouldnt that basically make it so that even if I instantiate Example as Example<{loading: false}, {}, ""> wouldn't the Exclude<{loading: false}, { [key: string]: boolean | true | false } & { [key: number]: boolean | true | false }> type exclude the conflicting loading property therefore allow the isLoading property to only resolve to the { loading: boolean }["boolean"] type ?

@weswigham
Copy link
Member

weswigham commented Sep 22, 2018

Because Exclude is imperfect and can't currently track constraints on the false branch of the conditional (in a generic fashion), as those are (effectively) subtraction types, which we can't easily express right now. :(

@weswigham
Copy link
Member

weswigham commented Sep 22, 2018

Extract does a little better (since we do track constraints in the true branch of a conditional type). If you can rephrase/reorder in terms of extract, you might manage the desired result.

@dmichon-msft
Copy link
Contributor

Excepting possible compilation performance issues, it's best not to use the intersection operator & to describe the result of the es6 spread operator, since later object spreads overwrite prior values. This is why I described your component type as:

ComponentType<{
  [PropKey in (keyof P | Key | 'loading' | 'error' | keyof MutationResult<TData>)]: PropKey extends Key ?
    MutationFn : PropKey extends keyof P ?
    P[PropKey] : PropKey extends keyof MutationResult<TData> ?
    MutationResult<TData>['data'][PropKey] : PropKey extends ('loading' | 'error') ?
    MutationResult<TData>[PropKey] : never;
}>

The value for [key: Key} MutationFn is written last, so takes precedence, then the values from this.props: P, then those from result.data: MutationResult<TData>['data'} then finally the results from result.loading: boolean and result.error: Error.

Incidentally, the compiler will flatten the above described type if you ever call it with concrete types, making it rather easier to find type mismatches.

Your example can also trivially be broken by calling:

const ex = new Example<{}, {}, "loading">();

which will also break at runtime, since ex.isLoading will (at runtime) be of type MutationFn

@IllusionElements
Copy link
Author

How does the Readonly type play into all this? It seems that the type only breaks when the intersection is of two Readonly types it will error out but if they aren't wrapped in Readonly it's completely fine

see here

@weswigham
Copy link
Member

The non-readonly case should also be an error; but for historical reasons it isn't. The presence of a generic mapped type (ie, read-only) causes us to defer the access and treat it as a type variable, which we can then actually calculate the constraint of and issue the error. The non-mapped-type case (incorrectly) eagerly looks up the type of the member and uses it as the target type. So instead of assigning to an effective T extends boolean as is the case of the access, you just assign to boolean. It's a known place where we're unsound, but don't change it for fear of breaking people's old code. :(

@IllusionElements
Copy link
Author

So what's the best way to write the type so that

 {  
[PropKey in
          | keyof P
          | Key
          | 'loading'
          | 'error'
          | keyof MutationResult<TData>]: PropKey extends Key
          ? MutationFn
          : PropKey extends keyof P
            ? P[PropKey]
            : PropKey extends keyof MutationResult<TData>
              ? Result<TData>['data'][PropKey]
              : PropKey extends ('loading' | 'error')
                ? { loading: boolean, error: Error }[PropKey]
                : never
      }["loading"] 

will resolve to boolean? and error would correctly resolve to the Error type?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs More Info The issue still hasn't been fully clarified
Projects
None yet
Development

No branches or pull requests

4 participants