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.2.2] `keyof` expanded to fixed properties in the ambient declaration of a mixin #29859

Open
SamuraiJack opened this Issue Feb 11, 2019 · 3 comments

Comments

Projects
None yet
3 participants
@SamuraiJack
Copy link

SamuraiJack commented Feb 11, 2019

Lets say we have some property in the class (FIELDS below), that "mirrors" all other properties using the keyof this, but with different fixed type. This enables nice type-safety when accessing properties of the FIELDS. The type of the FIELDS property in the generated declaration file is correct - it is same.

export class Field {}

export class Example {
    someProperty        : number
    anotherProperty     : string

    // collection of "fields"
    FIELDS              : { [s in keyof this] : Field }
}

const ex1       = new Example()

ex1.FIELDS.anotherProperty // valid
ex1.FIELDS.nonsense // invalid


// export declare class Example {
//     someProperty: number;
//     anotherProperty: string;
//     FIELDS: {
//         [s in keyof this]: Field;  ALL IS FINE HERE
//     };
// }

However, when same approach is used in the mixin class, the type of FIELDS in the generated declaration file is different - its been expanded from the keyof this to a fixed set of properties. Naturally this does not work for subclasses.

// supporting definitions for mixin pattern
export type AnyFunction<A = any>      = (...input: any[]) => A
export type AnyConstructor<A = any>   = new (...input: any[]) => A
export type Mixin<T extends AnyFunction> = InstanceType<ReturnType<T>>

// mixin function
export const SampleMixin = <T extends AnyConstructor<object>>(base : T) =>

class SampleMixin extends base {
    someProperty        : number

    anotherProperty     : string

    FIELDS              : { [s in keyof this] : Field }
}
// mixin type
export type SampleMixin = Mixin<typeof SampleMixin>


const cls       = SampleMixin(Object)

const ex2       = new cls

ex2.FIELDS.anotherProperty // valid
ex2.FIELDS.nonsense // invalid

// export declare const SampleMixin: <T extends AnyConstructor<any>>(base: T) => {
//     new (...input: any[]): {
//         [x: string]: any;
//         someProperty: number;
//         anotherProperty: string;
//         FIELDS: {        
//             [x: string]: Field;
//             someProperty: Field;      <-- `keyof this` TYPE EXPANDED
//             anotherProperty: Field;
//             FIELDS: Field;
//         };
//     };
// } & T;
// export declare type SampleMixin = Mixin<typeof SampleMixin>;

As a result of this, if you use mixins and such approach, it is not possible to use composite projects (--build), because the types are all incorrect.

And the irony is, that if you use mixins extensively (as we do in our project, because its a very clean and type-safe way to compose features), the compilation time significantly increases at some point, so we would really want to use the composite projects..

@RyanCavanaugh

This comment has been minimized.

Copy link
Member

RyanCavanaugh commented Feb 19, 2019

This seems more like an observation than a bug report or a suggestion. What's being proposed?

@SamuraiJack

This comment has been minimized.

Copy link
Author

SamuraiJack commented Feb 20, 2019

@RyanCavanaugh The expected and proposed behavior is of course to not expand the keyof in the mixin declarations - in the same way as it happens in regular classes.

@dragomirtitian

This comment has been minimized.

Copy link
Contributor

dragomirtitian commented Feb 20, 2019

Related issue: #27171

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.