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

[bug/generics]: Class Generics doesn't flow properly to subclass generics #25296

Closed
Hotell opened this issue Jun 28, 2018 · 6 comments
Closed
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@Hotell
Copy link

Hotell commented Jun 28, 2018

TypeScript Version: 2.9.2 + {strict:true}

Search Terms:
jsx generics, class generics

I wanna create React generic component. but the generic annotations flow doesn't work properly/is acting strange

Code

There are various issues with generics flow:

1. When I define Props member, which is type of function via property, following throws error:

image

[ts]
Argument of type 'Readonly<{ children?: ReactNode; }> & Readonly<Props<T>>' is not assignable to parameter of type 'Props<{}>'.
  Types of property 'onSelect' are incompatible.
    Type '(item: T, event?: SyntheticEvent<HTMLElement> | undefined) => void' is not assignable to type '(item: {}, event?: SyntheticEvent<HTMLElement> | undefined) => void'.
      Types of parameters 'item' and 'item' are incompatible.
        Type '{}' is not assignable to type 'T'.

but if i change it to "method" definition, error is gone

image

2. Generics default values doesn't flow properly:

image

throws

image

T = {} is not propagated correctly to Props and because of that we have those errors.

What partially helps is to constraining generic to string in this case, but event with that it won't flow to our Props['onSelect']

image

throws:

image

finally what fixes the issue is to cast value propagated to onSelect to our T

image

Expected behavior:

class generics should properly flow to subclasses generics

type Props<T extends string = string> = {
  items: T[]
  name: string
  onSelect(item: T, event?: SyntheticEvent<HTMLElement>): void
}
export class Select<T extends string = string> extends Component<Props<T>> {
  private readonly handleChange = (event: React.FormEvent<HTMLInputElement>) => {
    const { value, name } = event.currentTarget
    const isValueValid = this.props.items.find((item) => item === value)

    if (isValueValid) {
      this.props.onSelect(value)
    }
  }
}
@Hotell Hotell changed the title Class Generics doesn't properly flow to subclass generics [bug/generics]: Class Generics doesn't flow properly to subclass generics Jun 28, 2018
@mhegazy
Copy link
Contributor

mhegazy commented Jun 28, 2018

The first issue seems like the right behavior, your createListId expects a Props<{}> (because of the default type parameter), which uses the T in a contravarient position, and since {} is not assignable to T, you get the error.

The second case, i only see one error on this.props.onSelect(value) which seems right to me as well, you can not assign string to T extends string

@mhegazy mhegazy added the Working as Intended The behavior described is the intended behavior; this is not a bug label Jun 28, 2018
@Hotell
Copy link
Author

Hotell commented Jun 28, 2018

  1. hmm gotcha, but still this is confusing:
type Props<T extends string = string> = {
  items: T[]
  name: string
// error
-  onSelect: (item: T, event?: SyntheticEvent<HTMLElement>) => void
// no error
+  onSelect(item: T, event?: SyntheticEvent<HTMLElement>): void
}
  1. its T extends string = string which is string by default. is this "error" valid ? I'm little confused with this. The confusion may be even higher because VSCode doesn't show that our T(string) generic was properly applied on Props.

Any more explanations are more than welcome 👌:)

@mhegazy
Copy link
Contributor

mhegazy commented Jun 29, 2018

its T extends string = string which is string by default. is this "error" valid ? I'm little confused with this. The confusion may be even higher because VSCode doesn't show that our T(string) generic was properly applied on Props.

default type is of no consequence here. T is generic, i.e. it can be a more specific type than its base constraint. e,g, "foo" | "bar" and assigning a string to that is not allowed.

@Hotell
Copy link
Author

Hotell commented Jun 30, 2018

default type is of no consequence here. T is generic, i.e. it can be a more specific type than its base constraint. e,g, "foo" | "bar" and assigning a string to that is not allowed.

AHA ! right !

so with that solved what about the strange behaviour in 1):

type Props<T extends string = string> = {
  items: T[]
  name: string
// error
-  onSelect: (item: T, event?: SyntheticEvent<HTMLElement>) => void
// no error
+  onSelect(item: T, event?: SyntheticEvent<HTMLElement>): void
}

thanks !

@mhegazy
Copy link
Contributor

mhegazy commented Jul 2, 2018

--strcitFunctionTypes enforces stronger type checks when comparing function types, but does not apply to methods. see https://github.com/Microsoft/TypeScript/wiki/What's-new-in-TypeScript#strict-function-types

@Hotell
Copy link
Author

Hotell commented Jul 4, 2018

Oh wow ! Didn't know that this applies also for ambient definitions ! Thanks so much for making things clear , much appreciated. Closing now 👌✌️

@Hotell Hotell closed this as completed Jul 4, 2018
Hotell added a commit to Hotell/typescript-lib-starter that referenced this issue Aug 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

2 participants