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

mixin properties not checked #26286

Closed
ghost opened this issue Aug 7, 2018 · 5 comments
Closed

mixin properties not checked #26286

ghost opened this issue Aug 7, 2018 · 5 comments
Assignees
Labels
Bug A bug in TypeScript Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@ghost
Copy link

ghost commented Aug 7, 2018

TypeScript Version: master (as of #26283)

Code

/**
 * @template {new (...args: any[]) => any} T
 * @param {T} SuperClass
 */
function myMixin(SuperClass) {
    return class extends SuperClass {
        method() { return 0; }
    }
};

class Base {}
const Mixed = myMixin(Base);

const instance = new Mixed();
instance.method(); // Correctly typed in signature help
instance.meshod(); // But not checked

Run with allowJs, checkJs, and strict.

Expected behavior:

Error at meshod.

Actual behavior:

No error.

@ghost ghost added Bug A bug in TypeScript Salsa Domain: JSDoc Relates to JSDoc parsing and type generation checkJs Relates to checking JavaScript using TypeScript Domain: JavaScript The issue relates to JavaScript specifically labels Aug 7, 2018
@ghost ghost assigned sandersn Aug 7, 2018
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.1 milestone Aug 8, 2018
@sandersn
Copy link
Member

This is true of Typescript as well:

function mix<T extends new (...args: any[]) => any>(S: T) {
    return class extends S {
        f() { return 0; }
    }
};

class Base {}
const Mixed = mix(Base);

export const m = new Mixed();
m.f(); // Correctly typed in signature help
m.j(); // should error here, but doesn't

@sandersn sandersn removed Domain: JSDoc Relates to JSDoc parsing and type generation Salsa checkJs Relates to checking JavaScript using TypeScript Domain: JavaScript The issue relates to JavaScript specifically labels Aug 10, 2018
@sandersn sandersn changed the title JavaScript mixin properties not checked mixin properties not checked Aug 10, 2018
@ghost
Copy link
Author

ghost commented Aug 10, 2018

Looks like it has to do with the type T extends returning any (fixed if you change it to new (...args: any[]) => {}) -- though I think the actual T ought to (but isn't) used instead of its constraint when outside of mix?

@sandersn
Copy link
Member

Well, the reason that you see the apparent type of T is that the any comes from class extends S. S has type T, whose apparent type has a constructor with return type any, which is equivalent to extending any. That’s what allows you to access any property on m.

@sandersn sandersn added the Working as Intended The behavior described is the intended behavior; this is not a bug label Aug 13, 2018
@ghost
Copy link
Author

ghost commented Aug 13, 2018

I think the problem then is that we force the user to write any in that position rather than unknown, which is an error (Base constructor return type 'unknown' is not a class or interface type.)

@michaeljota
Copy link

I though about using InstanceType in the return, that actually makes sense, although the code does throw when expected, it also throws extending S

function mix<T extends new (...args: any[]) => InstanceType<T>>(S: T) {
    return class extends S { // Throws: Base constructor return type 'InstanceType<T>' is not a class or interface type.
        f() { return 0; }
    }
};

class Base {}
const Mixed = mix(Base);

export const m = new Mixed();
m.f(); // Correctly typed in signature help
m.j(); // Throws: Property 'j' does not exist on type 'mix<typeof Base>.(Anonymous class) & Base'.

Maybe I'm using InstanceType poorly, but I think this should be the way to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

3 participants