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

Compile errors with a generic base class and class mixins using declarative typings #14017

Closed
agubler opened this issue Feb 11, 2017 · 11 comments
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@agubler
Copy link

agubler commented Feb 11, 2017

TypeScript Version: 2.2.0 (rc) & 2.2.0-dev.20170211

I am having trouble creating consumable typings when using class mixins for a generic class from a core library - github example repro here https://github.com/agubler/typescript-mixin-typings.

Consider the following provided by a core library

// BaseClass.ts
export type Constructor<T> = new (...args: any[]) => T;

export class MyBaseClass<T> {
    baseProperty: string;

    constructor(value: T) {}
}
// MixinClass.ts
import { Constructor, MyBaseClass } from './BaseClass.ts';

export interface MyMixin {
    mixinProperty: string;
}

export function MyMixin<T extends Constructor<MyBaseClass<any>>>(base: T): T & Constructor<MyMixin> {
    mixinProperty: string;
}
// FinalClass
export class MyExtendedClass extends MyMixin(MyBaseClass)<string> {
	extendedClassProperty: number;
}

compiling with declarations true

{
		"declaration": true,
		"emitDecoratorMetadata": true,
		"experimentalDecorators": true,
		"module": "umd",
		"moduleResolution": "node",
		"noImplicitAny": true,
		"noImplicitThis": true,
		"removeComments": false,
		"sourceMap": true,
		"strictNullChecks": true,
		"target": "es5"
}

Usage in downstream consumer project:

import { MyExtendedClass } from 'core-library/FinalClass';
import { MyMixin } from 'core-library/MixinClass';

// constructor signature lost
const myExtendedClass = new MyExtendedClass('string');

// MyExtendedClass no longer type MyBaseClass
const AnotherMixedClass = MyMixin(MyExtendedClass);

Errors:
5 col 25 error| Supplied parameters do not match any signature of call target. [typescript/tsuquyomi]

8 col 35 error| Argument of type 'typeof MyExtendedClass' is not assignable to parameter of type 'Constructor<MyBaseClass<any>>'. Type 'MyExtendedClass' is not assignable to type 'MyBaseClass<any>'. Property 'baseProperty' is missing in type 'MyExtendedClass'. [typescript/tsuquyomi]

@ahejlsberg
Copy link
Member

Your example has several syntax errors and missing imports in it. However, once I fix it up the following compiles with no errors:

// BaseClass.ts
export type Constructor<T> = new (...args: any[]) => T;

export class MyBaseClass<T> {
    baseProperty: string;
    constructor(value: T) {}
}
// MixinClass.ts
import { Constructor, MyBaseClass } from './BaseClass';

export interface MyMixin {
    mixinProperty: string;
}

export function MyMixin<T extends Constructor<MyBaseClass<any>>>(base: T): T & Constructor<MyMixin> {
    return class extends base {
        mixinProperty: string;
    }
}
// FinalClass.ts
import { MyBaseClass } from './BaseClass';
import { MyMixin } from './MixinClass';

export class MyExtendedClass extends MyMixin(MyBaseClass)<string> {
    extendedClassProperty: number;
}
// Main.ts
import { MyExtendedClass } from './FinalClass';
import { MyMixin } from './MixinClass';

const myExtendedClass = new MyExtendedClass('string');

const AnotherMixedClass = MyMixin(MyExtendedClass);

@ahejlsberg ahejlsberg added the Working as Intended The behavior described is the intended behavior; this is not a bug label Feb 12, 2017
@agubler
Copy link
Author

agubler commented Feb 12, 2017

@ahejlsberg Thanks for looking. Was it the GitHub repro that was wrong? Or my the example code on the issue? Or both?

@agubler
Copy link
Author

agubler commented Feb 12, 2017

The honest answer is I want you to be correct. But I really struggled to get anything compiling given the 'd.ts' typings generated from core code in the example.

@agubler
Copy link
Author

agubler commented Feb 12, 2017

@ahejlsberg did you try main.ts compiling from the outputted typings rather than the typescript files in the same project? Because that is the important part to reproducing the issue, I assume that all functionality should be supported by the generated d.ts files when importing an external package? Here's a link to my repro: https://github.com/agubler/typescript-mixin-typings

Note: I see the copy/paste fail in the code I put on the issue (sorry about that).

@ahejlsberg
Copy link
Member

ahejlsberg commented Feb 12, 2017

Ah, I see what the problem is now. The declaration file emitter is emitting incorrect code in FinalClass.d.ts:

// FinalClass.d.ts
import { MyBaseClass } from './BaseClass';
import { MyMixin } from './MixinClass';
export declare class MyExtendedClass extends MyBaseClass<string> & MyMixin {
    extendedClassProperty: number;
}

The extends clause of a class needs to specify an expression, but the declaration file emitter emits a type. It should instead report an error if the expression isn't a simple variable.

You can work around the issue by introducing a const that represents the mixin base class:

// FinalClass.ts
import { Constructor, MyBaseClass } from './BaseClass';
import { MyMixin } from './MixinClass';

export const MyMixedBaseClass = MyMixin(MyBaseClass);

export class MyExtendedClass extends MyMixedBaseClass<string> {
	extendedClassProperty: number;
}

That produces the following correct output:

// FinalClass.d.ts
import { Constructor, MyBaseClass } from './BaseClass';
import { MyMixin } from './MixinClass';
export declare const MyMixedBaseClass: typeof MyBaseClass & Constructor<MyMixin>;
export declare class MyExtendedClass extends MyMixedBaseClass<string> {
    extendedClassProperty: number;
}

@ahejlsberg ahejlsberg added Bug A bug in TypeScript and removed Working as Intended The behavior described is the intended behavior; this is not a bug labels Feb 12, 2017
@agubler
Copy link
Author

agubler commented Feb 12, 2017

Great, thank you - I will try that out now (perhaps I should have just put the emitted code from FinalClass.d.s in the issue!).

Is it just a side effect that is works when a generic is not passed? Because without a generic the emitted code is

export declare class MyExtendedClass extends MyBaseClass & MyMixin {
    extendedClassProperty: number;
}

So still a type but behaves as expected?

@mhegazy mhegazy added this to the TypeScript 2.2.1 milestone Feb 13, 2017
@sandersn
Copy link
Member

@agubler extending an intersection still doesn't work though — it fails in the parser (and has no supporting code in the checker either). After some discussion, we're going to make it a d.ts error for 2.2 and consider supporting & in extends clauses in 2.3.

@agubler
Copy link
Author

agubler commented Feb 14, 2017

@sandersn Thanks for the feedback, the work around to assign the result of the mixins to a intermediary const is still fine though? As per @ahejlsberg's suggestion?

@sandersn
Copy link
Member

Yes, it's just a limitation of the parsing and checking of heritage clauses — they expect a type reference, not an arbitrary expression.

@agubler
Copy link
Author

agubler commented Feb 14, 2017

I thought as much, at least with it being an error the issue will be highlighted before we try to consume the published module(s).

@sandersn
Copy link
Member

Initial fix is up at #14074

I'll look at actually parsing intersections after this so that it's no longer an error.

@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Feb 14, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

4 participants