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

Unexpected error when using generic with Pick and Exclude #28748

Closed
OliverJAsh opened this issue Nov 29, 2018 · 21 comments
Closed

Unexpected error when using generic with Pick and Exclude #28748

OliverJAsh opened this issue Nov 29, 2018 · 21 comments
Assignees
Labels
Bug A bug in TypeScript Domain: Conditional Types The issue relates to conditional types

Comments

@OliverJAsh
Copy link
Contributor

OliverJAsh commented Nov 29, 2018

TypeScript Version: 3.2.1

Search Terms: generic pick exclude

Code

//
// Example 1
//

const fn = <Params extends {}>(
    params: Pick<Params, Exclude<keyof Params, never>>,
): Params =>
    // Unexpected error
    /* Type 'Pick<Params, Exclude<keyof Params, never>>' is not assignable to type 'Params'. */
    params;

//
// Example 2
//

import * as React from 'react';
import { ComponentType, FunctionComponent } from 'react';

type User = { name: string };
type UserProp = { user: User };

export const myHoc = function<ComposedComponentProps extends UserProp>(
    ComposedComponent: ComponentType<ComposedComponentProps>,
) {
    // These two should be equivalent, but they're not?
    // Doesn't work:
    type Props = Pick<
        ComposedComponentProps,
        Exclude<keyof ComposedComponentProps, never>
    >;
    // Works:
    // type Props = ComposedComponentProps;

    const Component: FunctionComponent<Props> = props => (
        // Unexpected error
        /* Type 'PropsWithChildren<Pick<ComposedComponentProps, Exclude<keyof ComposedComponentProps, never>>>' is not assignable to type 'IntrinsicAttributes & ComposedComponentProps & { children?: ReactNode; }'.
            Type 'PropsWithChildren<Pick<ComposedComponentProps, Exclude<keyof ComposedComponentProps, never>>>' is not assignable to type 'ComposedComponentProps'. */
        <ComposedComponent {...props} />
    );

    return Component;
};
@iiroj
Copy link

iiroj commented Nov 30, 2018

I am also having this issue with version 3.2.1. The following code used to work before:
https://github.com/iiroj/breakpoint-observer/blob/52baadc23179d959f047cc07a67c0954b692778a/index.tsx#L163

EDIT:

I was able to work around this issue like so:
https://github.com/iiroj/breakpoint-observer/commit/4d24c19f19e140542a6e077b88005777543cbcad

@deftomat
Copy link

@weswigham weswigham added Bug A bug in TypeScript Domain: Conditional Types The issue relates to conditional types labels Nov 30, 2018
@sledorze
Copy link

Same problems here too.
Lost a day finding workarounds in our big monorepo..

@Nerlin
Copy link

Nerlin commented Dec 3, 2018

Same problem for me.
Broke my typing:

export type Injector<P> = <T extends P>(Component: React.ComponentType<T>) => React.ComponentType<Subtract<T, P>>;
export type Subtract<T, K> = Omit<T, keyof K>;
export type Omit<T, K> = Pick<T, Exclude<keyof T, K>>;

const withContext: Injector<Context> = (Component) => (props) => (
  <ContextProvider>{(context) => <Component {...context} {...props} />}</ContextProvider>
);

@jkillian
Copy link

@sledorze just curious, did you find any satisfactory workarounds for this? Unfortunately it's preventing us from upgrading to TS 3.2 at the moment

@sledorze
Copy link

sledorze commented Dec 11, 2018

@jkillian not using Pick, not relying on key selection.

going from something like

export const myHoc = function<ComposedComponentProps extends UserProp>(
  ComposedComponent: ComponentType<ComposedComponentProps>,
) ... ```

to

```ts
export const myHoc = function<ComposedComponentProps>(
  ComposedComponent: ComponentType<ComposedComponentProps & UserProp>,
)

Unfortunately, the downside is that the types you are using must be some explicit intersection in order to infer the types correctly.

That means no more using interfaces.

I hope Typescript will provide some means to tackle this (maybe that's the case with the last version, but I'm slashing code on the backend ATM - so will not be using Typescript until 2019..).

@vict-shevchenko
Copy link

It will be great to hear from @typescript team if this is really a bug in 3.2.2 version or it means that previously our react HOC were typed incorrectly.

I have tried adopting a @iiroj proposed workaround, and it actually fixes TS error in HOC, but cause another issue. TS requires an injected prop on the wrapped component.

Example

type Omit<T, K> = Pick<T, Exclude<keyof T, K>>;
type Subtract<T, K> = Omit<T, keyof K>;

interface IName {
  name: string;
}

function withName<P extends IName>(Component: React.ComponentType<P>) {
  return class WithName extends React.Component<Subtract<P, IName>> {
    public render() {
      return (
        <NameContext.Consumer>
          {(name: string)  => <Component {...this.props} name={name} />}
                                            ^^^^^ Type 'Readonly<{ children?: ReactNode; }> & Readonly<Pick<P, Exclude<keyof P, "name">>> & { name: string; }' is not assignable to type 'P'.ts(2322)
        </NameContext.Consumer>
      );
    }
};

when I replace

function withName<P extends {}>(Component: React.ComponentType<P & IName>) {
  return class WithName extends React.Component<P> {
    public render() {
      return (
        <NameContext.Consumer>
          {(name: string)  => <Component {...this.props} name={name} />}
        </NameContext.Consumer>
      );
    }
};

const Named = withName(WitoutName);

<Named /> // TS Error - property 'name' is missing

I don't really want to make name prop on IName interface optional. As in 3.1.6 - TS returned a correct error for a case <Named name={'Test'} /> saying that name property is forbidden here.

@iiroj
Copy link

iiroj commented Dec 14, 2018

@vict-shevchenko see my example here: https://github.com/iiroj/breakpoint-observer/blob/master/stories/withBreakpoint.story.tsx

Here the <DisplayBreakpoint / requires some breakpoint props, but wrapping it with withBreakpoint correctly interprets that <CurrentBreakpoint /> has them, and that you can’t pass them manually to it.

@vict-shevchenko
Copy link

Hi, @iiroj, I have analyzed your example and made a small reproducible scenario based on it. Truthfully results are somehow confusing. I made a conclusion that your example work correctly only because you pass showMaxWidth. (also all props on type CurrentBreakpoint are optional)

Here is a code snippet:

import * as React from "react";

interface NameInterface {
  name: string;
};

function withName<P extends object>(Component: React.ComponentType<P & NameInterface>) {
  return function (props: P) {
    return (
      <Component
        {...props}
        name={'John'}
      />
    );
  }
}

const UnNamed_1: React.SFC<NameInterface> = ({ name }) => (<p>My name is {name}</p>);
const Named_1 = withName(UnNamed_1);
<Named_1 /> // Property 'name' is missing in type '{}' but required in type 'NameInterface'.ts(2741)


interface UnNamedPropsInterface extends NameInterface {
  age: number;
}
const UnNamed_2: React.SFC<UnNamedPropsInterface> = ({ name, age }) => (<p>My name is {name} and {age}</p>);
const Named_2 = withName(UnNamed_2);
<Named_2 age={30} /> // Property 'name' is missing in type '{ age: number; }' but required in type 'UnNamedPropsInterface'.ts(2741)


type UnNamedType = NameInterface;
const UnNamed_3: React.SFC<UnNamedType> = ({ name }) => (<p>My name is {name}</p>);
const Named_3 = withName(UnNamed_3);
<Named_3 /> // Property 'name' is missing in type '{}' but required in type 'NameInterface'.ts(2741)


type UnNamedType_2 = NameInterface & { age: number };
const UnNamed_4: React.SFC<UnNamedType_2> = ({ name, age }) => (<p>My name is {name} and {age}</p>);
const Named_4 = withName(UnNamed_4);
<Named_4 age={30} /> // All OK, but why?

Can someone please advise.
Thanks.

@iiroj
Copy link

iiroj commented Dec 15, 2018

Ah, so I see mine’s not really workaround but just just happens to work because of the optional props (window, and thus width doesn’t exist during SSR).

@OliverJAsh
Copy link
Contributor Author

OliverJAsh commented Dec 15, 2018

Updated my original post with a much simpler example:

const fn = <Params>(
    params: Pick<Params, Exclude<keyof Params, never>>,
): Params => params;

@aprilandjan
Copy link

Also spent nearly one whole day and found this issue... How dumb I am 😭

@ramondeklein
Copy link

ramondeklein commented Dec 29, 2018

@aprilandjan You're not alone :-) I was following some examples and I just didn't get it to work. I thought I was doing something wrong, but it broke with TypeScript v3.2.

I have found a workaround to "fix" this issue. It's a fairly simple fix and you can apply it by spreading the properties as any instead of the typed component. You keep all type-checking on the outside, so it's a fairly non-intrusive fix. Just change:

return (<WrappedComponent {...this.props} locale={locale} />);

into

return (<WrappedComponent {...(this.props as any)} locale={locale} />);

and the error is gone.

@dupski
Copy link

dupski commented Dec 29, 2018

Hi Guys. Heres my complete (simplified) example which worked with TS 3.1 but does not work with TS 3.2

import * as React from "react"

const AuthContext = React.createContext({})

export type Omit<T, K extends string> = Pick<T, Exclude<keyof T, K>>

export interface IAuthContext {
    currentUser: string
}

export interface IAuthContextProp {
    auth: IAuthContext
}

export function withAuthContext<
    TComponentProps extends IAuthContextProp,
    TWrapperProps = Omit<TComponentProps, keyof IAuthContextProp>
>(
    Component: React.ComponentType<TComponentProps>
): React.ComponentType<TWrapperProps> {
    return (props: TWrapperProps): React.ReactElement<TComponentProps> => (
        <AuthContext.Consumer>{(auth) => (
            <Component auth={auth} {...props} />
        )}</AuthContext.Consumer>
    )
}

The error in TS 3.2 (on the <Component auth= ... line) is:

Type '{ auth: {}; } & TWrapperProps' is not assignable to type 'IntrinsicAttributes & TComponentProps & { children?: ReactNode; }'.
  Type '{ auth: {}; } & TWrapperProps' is not assignable to type 'TComponentProps'. ts(2322)

Thanks for your workaround @ramondeklein , it works nicely.
In my case I simply changed return (props: TWrapperProps) to return (props: any)

I wonder if this could be a problem with the React typings? For reference I am on "@types/react": "16.7.6"

@RyanCavanaugh RyanCavanaugh removed this from the TypeScript 3.4.0 milestone Feb 1, 2019
@kiliancs
Copy link

kiliancs commented Feb 2, 2019

Is this the same problem? What we are doing has really nothing to do with my example here, but it is the simpler way to reproduce the same problem that I found.

interface ValueWrapper<V> {
    value: V;
}

interface StringValue extends ValueWrapper<string> {
    fontSize: number;
}

const foo: Exclude<StringValue, ValueWrapper<string>> = {
    // Type 'number' is not assignable to type 'never'. [2322]
    fontSize: 2
};

Used to work with TypeScript 3.1.1 and is preventing us from upgrading.

@toolness
Copy link

Hmm, so it looks like fixing this was on the roadmap for TS 3.3, but then it got removed, and it was briefly on the milestone for 3.4 but got removed from that too, so I'm not sure when this will be addressed. 😞

That said, I ended up using the workaround proposed in #28748 (comment). While seeing props as any made me worried, as @ramondeklein mentioned, all type-checking is maintained on the outside interface to your code, so depending on the structure of your code, you might still be getting type safety where it matters most. In my case I also left a comment in the source linking to this issue so that readers know it's a workaround for something that will hopefully eventually be fixed.

@OliverJAsh
Copy link
Contributor Author

The original examples are fixed in 3.4.1 (by #29437?), however some issues seem to remain:

const fn = <Params, Key extends keyof Params>(
    params: Pick<Params, Key> & Pick<Params, Exclude<keyof Params, Key>>
): Params =>
    // Unexpected error
    // Type 'Pick<Params, Key> & Pick<Params, Exclude<keyof Params, Key>>' is not assignable to type 'Params'.
    params;

I guess this is a separate issue? /cc @weswigham @RyanCavanaugh

@karol-majewski
Copy link

@OliverJAsh Isn't that related to #28884?

@weswigham
Copy link
Member

Aye.

@sophister
Copy link

sophister commented Jun 13, 2019

same problem +1

TypeScript version: 3.4.3

@maxizhukov
Copy link

Still same issue with running yarn but npm works fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: Conditional Types The issue relates to conditional types
Projects
None yet
Development

No branches or pull requests