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

Exclude doesn't work with extended generics #24791

Closed
niba opened this issue Jun 8, 2018 · 7 comments
Closed

Exclude doesn't work with extended generics #24791

niba opened this issue Jun 8, 2018 · 7 comments
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@niba
Copy link

niba commented Jun 8, 2018

TypeScript Version: 2.9.1, 3.0.0-dev.20180608

Search Terms:
keyof generics extend exclude
Code

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

interface RequiredFields {
    requiredField: string;
    requiredField2: string;
}

// works
interface MyInterface extends RequiredFields { }

type ExcludeTest = Exclude<MyInterface, "">; 
type Fields = Omit<MyInterface, "">;
const test = (props: Fields) => {
    const { requiredField } = props;
}

// doesnt work

const generator = <T extends RequiredFields>() => {
    type ExcludeTest = Exclude<T, "">; // wrong 
    type PickTest = Pick<T, "requiredField">;

    type Fields = Omit<T, "">;
    const tmp = (props: Fields) => {
        const {  } = props; // requiredField doesnt exist
    }
    return tmp;
}

Expected behavior:

Props in generator should have RequiredFields fields.

Actual behavior:

Props are empty.

Playground Link: Link
Related Issues:
#21941 #21862.
The issues are closed but the bug is still here

@niba niba changed the title Keyof on extended generics doesn't work Exclude with extended generics doesn't work Jun 8, 2018
@niba niba closed this as completed Jun 8, 2018
@niba niba reopened this Jun 8, 2018
@niba niba changed the title Exclude with extended generics doesn't work Exclude doesn't work with extended generics Jun 8, 2018
@poseidonCore
Copy link

I get a similar issue with

// A basic record:
class R { 
    s: string;
    n: number;
    b: boolean;
}

// A field definition:
class F<IR extends R = R> { 
    name: keyof IR;
    type: "string" | "number" | "boolean";
}

// A function that operates over a record, taking either a field definition or a field name:
function doThis<IR extends R>(options:{
    fieldName?: string;
    field?: F<IR>
}) { 
    var fieldName: string = options.fieldName || options.field.name; // -> ERROR quoted below.

    //...
}

ERROR:

Type 'string | keyof IR' is not assignable to type 'string'.
Type 'keyof IR' is not assignable to type 'string'.
Type 'string | number | symbol' is not assignable to type 'string'.
Type 'number' is not assignable to type 'string'.

If I allow TS to infer the type of fieldName, then it has this type string | keyof IR, but this is not an extension of string, which can be shown by trying to use a string method like fieldName.length, which gives an error.

This worked fine in TS2.8, but the algorithm for comprehending keyof seems to be going awry.

This issue can be circumvented by using

class F<IR extends R = R> { 
    name: Exclude<keyof IR, number | symbol>;
    type: "string" | "number" | "boolean";
}

instead, but my worry is that based on the stated new keyof algorithm, should I really have to put in that filter given that keyof IR extends R = R would normally imply that in this situation, there are only string keys anyway in IR?

I understand that there might be a later class S that extends R and includes keys that are numbers|symbols, and could be used for IR, but that does not seem like a practical rational for this algorithm widening.

... Perhaps I am missing something ...

When I put in these Exclude circumventions, there seems to be a cascade of knock-on issues throughout my solution. I have had to revert to TS2.8 to avoid this.

Thanks

@tanglebones
Copy link

I've run into a similar case on [email protected]:

Playground Link

function bindp<T extends object, P extends object, R>(
  f: (T) => R,
  p: P
):
  (q: Exclude<T, P>) => R {
  return function (q: Exclude<T, P>): R {
    const args: T = <T>{...<object>q, ...<object>p}; // casts shouldn't be required here?
    return f(args);
  }
}

function f({a, b}: { a: string, b: number }): string {
  return a + b;
}

const g = bindp(f, {a: "a"});

console.log(g({b:1})); // -> "a1"
console.log(g({})); // should be an compile error?

@mhegazy
Copy link
Contributor

mhegazy commented Jul 18, 2018

Seems to be the same as #22137 and should be addressed by #22348

@mhegazy mhegazy added the Bug A bug in TypeScript label Jul 18, 2018
@mhegazy mhegazy added this to the TypeScript 3.1 milestone Jul 18, 2018
@tanglebones
Copy link

My understanding of Exclude was wrong, but my redone version still needs casts.

This may be the correct way to write it, but is still runs into the as any as object and as object casts for q and p.

export function bindp<
  P extends object, // subset of params to bind
  T extends P, // full set of params for f
  R, // return type of f
  Q = { [x in Exclude<keyof T, keyof P>]: T[x] } // all params in T less those in P
  >(
  f: (t: T) => R, // function to bind
  p: P, // params to bind
): (q: Q) => R { // returns a function that takes Q and returns R
  return function (q: Q): R {
    const args: T = {... q as any as object, ... p as object} as T; // merge q and p into object args of type T
    return f(args);
  };
}

function f({a, b}: { a: string, b: number }): string {
  return a + b;
}

const g = bindp(f, {a: "a"});

console.log(g({b: 1})); // -> "a1"
console.log(g({})); // compile error.

@MastroLindus
Copy link

MastroLindus commented Mar 12, 2019

I have a related case that worked perfectly in typescript 3.1 (or at least wasn't giving errors), and stopped working from typescript 3.2 onwards (including latest 3.4 beta).
This is an example for a higher order function that takes a React component as input, and returns a component that take the same props as the first one, minus a few props that get injected from the React context.

type ContextProps = {contextProp1: string, contextProp2: string};

type GetOutputProps<T extends Partial<ContextProps>> = Pick<T, Exclude<keyof T, keyof ContextProps>>;

export function contextPropInjector<InputProps extends Partial<ContextProps>>(Component: React.ComponentType<InputProps>): React.ComponentType<GetOutputProps<InputProps>> {
  return (props: GetOutputProps<InputProps>) => <myContext.Consumer>
    {(contextProps: ContextProps) => <Component {...props} {...contextProps} />}
  </myContext.Consumer>;
}

The error is that {...props, ...contextProps} is not assignable to InputProps, when it should be.
I use Partial in the example, but it still doesn't work when using the non-partial type.

@jack-williams
Copy link
Collaborator

@tanglebones You need to write your example like this:

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

function bindp<T extends P, P extends object, R>(
  f: (arg: T) => R,
  p: P
):
  (q: Omit<T,keyof P>) => R {
  return function (q: Omit<T,keyof P>): R {
    const args: T = <T>{...q, ...p};
    return f(args);
  }
}

@weswigham
Copy link
Member

fixgif

Issue described by the OP seems to be fixed - we did update generic mapped type constraints to be more correct a bit ago.

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

9 participants