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

[3.5.0-dev.20190516] Incorrect type error for mixin #31426

Open
canonic-epicure opened this issue May 16, 2019 · 4 comments
Open

[3.5.0-dev.20190516] Incorrect type error for mixin #31426

canonic-epicure opened this issue May 16, 2019 · 4 comments
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue
Milestone

Comments

@canonic-epicure
Copy link

When using the mixin pattern, there are 2 main notations to define the type of the mixin entity.
The mixin pattern:

export type AnyFunction<A = any>        = (...input : any[]) => A
export type AnyConstructor<A = object>  = new (...input : any[]) => A
export type Mixin<T extends AnyFunction> = InstanceType<ReturnType<T>>

export const Box = <T extends AnyConstructor<object>>(base : T) =>
class Box extends base {
    value       : any
}

1st notation:

export type Box = Mixin<typeof Box> {}

2nd notation:

export interface Box extends Mixin<typeof Box> {}

The 1st notation can not be used for recursive definitions (#29872). Because of that we primarily use 2nd notation. It works fine in most cases, however I found a case, when it produces invalid type error. The full snippet to reproduce the problem below.

Note:

  • The typechecker correctly figures out that there's no zxc property on this, inside the observe method of Quark mixin.
  • In that method, it does not complain about the this.value usage
  • It does complain, when value is used on function argument
  • If you'll switch the Quark mixin to the 1st notation, the error will disappear

Expected behavior:

  • No type errors for the definition of test function below
export type AnyFunction<A = any>        = (...input : any[]) => A
export type AnyConstructor<A = object>  = new (...input : any[]) => A
export type Mixin<T extends AnyFunction> = InstanceType<ReturnType<T>>

export const Box = <T extends AnyConstructor<object>>(base : T) =>
class Box extends base {
    value       : any
}
export interface Box extends Mixin<typeof Box> {}

export const Observable = <T extends AnyConstructor<object>>(base : T) =>
class Observable extends base {
    observe () : Quark {
        return
    }
}
export interface Observable extends Mixin<typeof Observable> {}

export const Quark = <T extends AnyConstructor<Box & Observable>>(base : T) =>
class Quark extends base {

    observe () : Quark {
        // No error here!
        this.value
        
        // error: Error:(28, 14) TS2339: Property 'zxc' does not exist on type 'Quark'.
        this.zxc
        
        return
    }
}
export interface Quark extends Mixin<typeof Quark> {}

const test = (a : Quark) => a.value // <-- Error:(35, 28) TS2339: Property 'value' does not exist on type 'Quark'.
@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Jun 13, 2019
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.6.0 milestone Jun 13, 2019
@orta
Copy link
Contributor

orta commented Aug 8, 2019

Interesting, so .value occurs in the intellisense for a Quark in these examples, perhaps the error is overly greedy here.

@orta orta added Bug A bug in TypeScript Fix Available A PR has been opened for this issue and removed Needs Investigation This issue needs a team member to investigate its status. labels Aug 8, 2019
@canonic-epicure
Copy link
Author

Glad to see the progress on this issue!

@canonic-epicure
Copy link
Author

@orta Perhaps, while you are in the context, you can check why the 2nd notation (#29872) is needed at all? There are some other problems with it, for example, casting a class type to interface loses the static methods information.

@orta
Copy link
Contributor

orta commented Feb 13, 2020

Putting this issue into backlog instead of milestoned, figuring out this issue was first evaluated in #32770 and then a more permentent fix was explored in #33050 (comment) but was considered too expensive on perf for the feature.

@orta orta removed their assignment Feb 13, 2020
@orta orta removed this from the TypeScript 3.9.0 milestone Feb 13, 2020
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Sep 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants