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

Type checking regression in mithril #28388

Closed
ghost opened this issue Nov 7, 2018 · 2 comments
Closed

Type checking regression in mithril #28388

ghost opened this issue Nov 7, 2018 · 2 comments
Assignees
Labels
Bug A bug in TypeScript Domain: This-Typing The issue relates to providing types to this

Comments

@ghost
Copy link

ghost commented Nov 7, 2018

TypeScript Version: 3.2.0-dev.20181106

Code

interface Lifecycle<Attrs, State> {
	oninit?(vnode: Vnode<Attrs, State>): number;
	[_: number]: any;
}

interface Vnode<Attrs, State extends Lifecycle<Attrs, State> = Lifecycle<Attrs, State>> {
	tag: Component<Attrs, State>;
}

interface Component<Attrs, State> {
	view(this: State, vnode: Vnode<Attrs, State>): number;
}

interface ClassComponent<A> extends Lifecycle<A, ClassComponent<A>> {
	oninit?(vnode: Vnode<A, this>): number;
	view(vnode: Vnode<A, this>): number;
}

interface Attrs { id: number }
class C implements ClassComponent<Attrs> {
	view(v: Vnode<Attrs>) { return 0; }
}

const test8: ClassComponent<any> = new C();

Expected behavior:

No error, as in typescript@3.1.

Actual behavior:

src/a.ts:24:7 - error TS2322: Type 'C' is not assignable to type 'ClassComponent<any>'.
  Types of property 'view' are incompatible.
    Type '(v: Vnode<Attrs, Lifecycle<Attrs, Lifecycle<Attrs, State>>>) => number' is not assignable to type '(vnode: Vnode<any, ClassComponent<any>>) => number'.
      Types of parameters 'v' and 'vnode' are incompatible.
        Type 'Vnode<any, ClassComponent<any>>' is not assignable to type 'Vnode<Attrs, Lifecycle<Attrs, Lifecycle<Attrs, State>>>'.
          Type 'ClassComponent<any>' is not assignable to type 'Lifecycle<Attrs, Lifecycle<Attrs, State>>'.
            Types of property 'oninit' are incompatible.
              Type '((vnode: Vnode<any, ClassComponent<any>>) => number) | undefined' is not assignable to type '((vnode: Vnode<Attrs, Lifecycle<Attrs, State>>) => number) | undefined'.
                Type '(vnode: Vnode<any, ClassComponent<any>>) => number' is not assignable to type '(vnode: Vnode<Attrs, Lifecycle<Attrs, State>>) => number'.
                  Types of parameters 'vnode' and 'vnode' are incompatible.
                    Type 'Vnode<Attrs, Lifecycle<Attrs, State>>' is not assignable to type 'Vnode<any, ClassComponent<any>>'.
                      Type 'Lifecycle<Attrs, State>' is not assignable to type 'ClassComponent<any>'.
                        Property 'view' is missing in type 'Lifecycle<Attrs, State>'.

24 const test8: ClassComponent<any> = new C();
         ~~~~~


Found 1 error.

Found in mithril on DefinitelyTyped.

@ghost ghost added the Bug A bug in TypeScript label Nov 7, 2018
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 3.2 milestone Nov 8, 2018
@weswigham weswigham added the Domain: This-Typing The issue relates to providing types to this label Nov 9, 2018
@weswigham
Copy link
Member

weswigham commented Nov 9, 2018

If you rename the Attrs interface to MyAttrs or the like, the error becomes a little easier to process:

!!! error TS2322: Type 'C' is not assignable to type 'ClassComponent<any>'.
!!! error TS2322:   Types of property 'view' are incompatible.
!!! error TS2322:     Type '(v: Vnode<MyAttrs, Lifecycle<MyAttrs, Lifecycle<Attrs, State>>>) => number' is not assignable to type '(vnode: Vnode<any, ClassComponent<any>>) => number'.
!!! error TS2322:       Types of parameters 'v' and 'vnode' are incompatible.
!!! error TS2322:         Type 'Vnode<any, ClassComponent<any>>' is not assignable to type 'Vnode<MyAttrs, Lifecycle<MyAttrs, Lifecycle<Attrs, State>>>'.
!!! error TS2322:           Type 'ClassComponent<any>' is not assignable to type 'Lifecycle<MyAttrs, Lifecycle<Attrs, State>>'.
!!! error TS2322:             Types of property 'oninit' are incompatible.
!!! error TS2322:               Type '(vnode: Vnode<any, ClassComponent<any>>) => number' is not assignable to type '(vnode: Vnode<MyAttrs, Lifecycle<Attrs, State>>) => number'.
!!! error TS2322:                 Types of parameters 'vnode' and 'vnode' are incompatible.
!!! error TS2322:                   Type 'Vnode<MyAttrs, Lifecycle<Attrs, State>>' is not assignable to type 'Vnode<any, ClassComponent<any>>'.
!!! error TS2322:                     Type 'Lifecycle<Attrs, State>' is not assignable to type 'ClassComponent<any>'.
!!! error TS2322:                       Property 'view' is missing in type 'Lifecycle<Attrs, State>'.

The signature type is reported as (v: Vnode<MyAttrs, Lifecycle<MyAttrs, Lifecycle<Attrs, State>>>) => number but should be closer to (v: Vnode<MyAttrs, Lifecycle<MyAttrs, (circular any placeholder)>>) => number. Seems like wee're defaulting to a constraint somewhere where we used to default to any instead. (note the uninstantiated Attrs and State params that have leaked!)

@weswigham
Copy link
Member

Yup, probably caused by #28222. I'll have a fix that handles the circular constraint more gracefully in a bit.

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: This-Typing The issue relates to providing types to this
Projects
None yet
Development

No branches or pull requests

2 participants