Skip to content

Mixin does not allow Generic #26154

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

Closed
Novaal opened this issue Aug 2, 2018 · 12 comments
Closed

Mixin does not allow Generic #26154

Novaal opened this issue Aug 2, 2018 · 12 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@Novaal
Copy link

Novaal commented Aug 2, 2018

TypeScript Version: 3.1.0-dev.20180802

Search Terms:

  • Typescript Mixins Generics
  • Base class expressions cannot reference class type parameters.

Code

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

interface BasicTypes {
  user: {name: string}
}

class BasicHandler<E extends BasicTypes> {
    // some methods here
}

interface TypesA extends BasicTypes {
   user: BasicTypes["user"] & {img: string}
}

interface TypesB extends BasicTypes {
    user: BasicTypes["user"] & {email?: string}
}

function UserMixin<E extends BasicTypes, B extends Constructor<BasicHandler<E>>>(Base: B)  {
    class UserHandler extends Base { //
        constructor(...args: any[]) {
            super(...args);
        }
        setUser(user: E["user"]) {
            //...
        }
    }
    return UserHandler;
}

// error TS2562: Base class expressions cannot reference class type parameters.
class HandlerA<MergedTypes extends TypesA> extends UserMixin<MergedTypes, Constructor<BasicHandler<MergedTypes>>>(BasicHandler) {
   // some other specific methods here
}

// error TS2562: Base class expressions cannot reference class type parameters.
class HandlerB<MergedTypes extends TypesB> extends UserMixin<MergedTypes, Constructor<BasicHandler<MergedTypes>>>(BasicHandler) {
    // some other specific methods here
 }

type mergedTypes = TypesA & TypesB

const currentUser: mergedTypes["user"] = {
    name: "Peter Parker",
    img: "https://....",
    email: "pp@example.org"
}

const a: HandlerA<mergedTypes> = {} as any;
a.setUser(currentUser)

const b: HandlerB<mergedTypes> = {} as any;
b.setUser(currentUser);

Expected behavior:
Mixins should allow generic types and propagate them to the inherited class

Actual behavior:
Cannot set generic type to mixin

Playground Link: Link

Related Issues:

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Aug 3, 2018
@RyanCavanaugh
Copy link
Member

This is intentional. See #17829 and #17922 , especially #17829 (comment)

@rzvc
Copy link

rzvc commented Aug 5, 2018

@RyanCavanaugh, what is the reason why it is considered that the inheritance list should be outside of the scope of the class declaration? (which is the root of this problem)

It's very counter intuitive and, right now, I can't think of any reason why it should be like that.

@RyanCavanaugh
Copy link
Member

@rzvc the type parameters of a class are not in scope in the static side of its declaration -- this should be noncontroversial as the static side of a class is a singleton and cannot vary over any type parameters (there is only one constructor function, not one class per instantiated type).

It follows that instance-specific type parameters are not in scope in the base class expression, because the base class expression is only evaluated once -- when Foo<T> extends SomeExpr, SomeExpr is only evaluated once and cannot vary over T because there isn't a T yet.

@rzvc
Copy link

rzvc commented Aug 6, 2018

@RyanCavanaugh, I'm sorry but I still don't get it. I'm not sure what you mean by the static side, as in my mind, it's all static (doesn't matter what types you feed in, you get the same code - I'm probably misunderstanding what you mean). You seem to imply that something would have to vary and I'm not sure what.

Can you please provide an explanation on the following code, on why exactly should T not be passed to test<T>(), in the inheritance list of Foo, when the type checker does its thing?

function test<T>(): new () => T
{
    return null;
}

class Foo<T> extends test<T>()
{
	public constructor()
	{
		super();
	}
}

@RyanCavanaugh
Copy link
Member

@rzvc let's say you wrote

new Foo<string>();
new Foo<number>();

How many times is test invoked?

The answer is 1, because test only gets invoked once, during the initialization of the class declaration.

This is different from Foo<T> - its constructor runs twice and can plausibly be seeing actual values of type T.

So it makes no sense for string and number to be provided at test, because there's no possible way that test is actually generic over T - its behavior cannot vary over its type parameter, because it can never deal with an actual value of T during its lifetime.

Put another way, let's say you wrote this:

function test<T>(argument: T): new () => T
{
    return null;
}

class Foo<T> extends test<T>(/* provide an argument */)
{
	public constructor()
	{
		super();
	}
}

There's nothing to "fill in the blank" with - test is lying about being generic; it doesn't actually vary over its argument because it has no arguments.

@rzvc
Copy link

rzvc commented Aug 7, 2018

@RyanCavanaugh, ok, I see what you mean, but it's not the whole story. What we really want to have as a generic is the return type, which is a constructor signature and gets called every time the class is instantiated.

Perhaps the type checker could reject the function call if it has generic parameters, that rely on the type of the base class, but not if just the return type does. In the end what we care about is for the returned constructor signature to be dependent on T.

Edit: Rephrased something.

@tedchirvasiu
Copy link

What seems weird to me is that this is okay:

class Foo<T> { }

class Bar<T> extends Foo<T> { }

Passing generics to anonymous classes is also okay:

function generateFoo() {
    return class<T> { }
}

const Foo = generateFoo();

class Bar<T> extends Foo<T> { }

However if we try to make the anonymous class inherit from a base class received as a parameter (mixin style), it breaks:

class Baz { }

export type Constructor<T = {}> = new (...args: any[]) => T;

function generateFoo<T extends Constructor<{}>>(Base: T) {
    return class<B> extends Base {
        constructor(...args: any[]) {
            super(...args);
        }
    }
}

const Foo = generateFoo(Baz);

class Bar<T> extends Foo<T> { }

Right of the bat it complains that "A mixin class must have a constructor with a single rest parameter of type 'any[]'" (which it does). It's the generic B that breaks it.

In my mind the result of generateFoo(Baz) should still be:

class Foo<B> extends Baz { }

Y u do dis typscrip

@Tundon
Copy link

Tundon commented Jun 7, 2019

@RyanCavanaugh

let's say you wrote

new Foo<string>();
new Foo<number>();

How many times is test invoked?

The answer is 1

Correct me if I am wrong, but if the similar generics code is in c++, there will definitely be two Foo classes, and test() would be invoked twice (I know c++ doesn't have this mixin mechanism, just assume similar mechanism exist in c++).
Would this be where the problem come from?

@Brain2000
Copy link

Brain2000 commented Jul 2, 2019

We are running into the same issues trying to create very low level React mixins for specialized components.

In other languages, including C++ generics become separate "copies". I believe C++ called them templates at one point.

Not only could this solve a number of issues that we are having, but it would also allow static properties to access generics, along with keeping a separate static property value per generic type, and perhaps other benefits as well.

@tannerntannern
Copy link

I know I'm a little late to the party, but ts-mixer might be able to help. It has a decent solution to the generics problem.

dgp1130 added a commit to dgp1130/ctor-exp that referenced this issue Nov 8, 2020
This mostly works as expected, with the one caveat that constructing a generic `Foo<T>` with `ctor.new(Foo)` actually results in a return-only generic. TypeScript is a little awkward about these and it will actually be typed as `Foo<unknown>`. This is unfortunate, and the best practice around return-only generics is generally considered casting them to the desired type at each call site. This means that a user should really do `ctor.new(Foo) as ctor<Foo<T>>`, which is a bit annoying syntatically, but not that big a deal in the grand scheme of things.

More annoyingly is that TypeScript does not allow type parameters in the `extends` clause of a class when used as an expression. This makes sense for TypeScript because a class type `T` is not known until the class is instantiated, long after it was declared as extending a different class. However, with the recent changes in `ctor<T>`, the inheritance hierarchy is not built until construction-time, so we actually do know the value of `T`. Unfortunately this can't be expressed in TypeScript, so our best option is to simply `@ts-ignore` the error. This is not ideal, but TypeScript *mostly* does what we want in this case. The one issue here is that superclass members are typed as `any`, which is quite unfortunate but I don't see a good way around that.

See: microsoft/TypeScript#26154 (comment)
dgp1130 added a commit to dgp1130/ctor-exp that referenced this issue Nov 8, 2020
This mostly works as expected, with the one caveat that constructing a generic `Foo<T>` with `ctor.new(Foo)` actually results in a return-only generic. TypeScript is a little awkward about these and it will actually be typed as `Foo<unknown>`. This is unfortunate, and the best practice around return-only generics is generally considered casting them to the desired type at each call site. This means that a user should really do `ctor.new(Foo) as ctor<Foo<T>>`, which is a bit annoying syntatically, but not that big a deal in the grand scheme of things.

More annoyingly is that TypeScript does not allow type parameters in the `extends` clause of a class when used as an expression. This makes sense for TypeScript because a class type `T` is not known until the class is instantiated, long after it was declared as extending a different class. However, with the recent changes in `ctor<T>`, the inheritance hierarchy is not built until construction-time, so we actually do know the value of `T`. Unfortunately this can't be expressed in TypeScript, so our best option is to simply `@ts-ignore` the error. This is not ideal, but TypeScript *mostly* does what we want in this case. The one issue here is that superclass members are typed as `any`, which is quite unfortunate but I don't see a good way around that.

See: microsoft/TypeScript#26154 (comment)
dgp1130 added a commit to dgp1130/ctor-exp that referenced this issue Nov 8, 2020
This mostly works as expected, with the one caveat that constructing a generic `Foo<T>` with `ctor.new(Foo)` actually results in a return-only generic. TypeScript is a little awkward about these and it will actually be typed as `Foo<unknown>`. This is unfortunate, and the best practice around return-only generics is generally considered casting them to the desired type at each call site. This means that a user should really do `ctor.new(Foo) as ctor<Foo<T>>`, which is a bit annoying syntatically, but not that big a deal in the grand scheme of things.

More annoyingly is that TypeScript does not allow type parameters in the `extends` clause of a class when used as an expression. This makes sense for TypeScript because a class type `T` is not known until the class is instantiated, long after it was declared as extending a different class. However, with the recent changes in `ctor<T>`, the inheritance hierarchy is not built until construction-time, so we actually do know the value of `T`. Unfortunately this can't be expressed in TypeScript, so our best option is to simply `@ts-ignore` the error. This is not ideal, but TypeScript *mostly* does what we want in this case. The one issue here is that superclass members are typed as `any`, which is quite unfortunate but I don't see a good way around that.

See: microsoft/TypeScript#26154 (comment)
dgp1130 added a commit to dgp1130/ctor-exp that referenced this issue Nov 8, 2020
This mostly works as expected, with the one caveat that constructing a generic `Foo<T>` with `ctor.new(Foo)` actually results in a return-only generic. TypeScript is a little awkward about these and it will actually be typed as `Foo<unknown>`. This is unfortunate, and the best practice around return-only generics is generally considered casting them to the desired type at each call site. This means that a user should really do `ctor.new(Foo) as ctor<Foo<T>>`, which is a bit annoying syntatically, but not that big a deal in the grand scheme of things.

More annoyingly is that TypeScript does not allow type parameters in the `extends` clause of a class when used as an expression. This makes sense for TypeScript because a class type `T` is not known until the class is instantiated, long after it was declared as extending a different class. However, with the recent changes in `ctor<T>`, the inheritance hierarchy is not built until construction-time, so we actually do know the value of `T`. Unfortunately this can't be expressed in TypeScript, so our best option is to simply `@ts-ignore` the error. This is not ideal, but TypeScript *mostly* does what we want in this case. The one issue here is that superclass members are typed as `any`, which is quite unfortunate but I don't see a good way around that.

See: microsoft/TypeScript#26154 (comment)
dgp1130 added a commit to dgp1130/ctor-exp that referenced this issue Nov 8, 2020
This mostly works as expected, with the one caveat that constructing a generic `Foo<T>` with `ctor.new(Foo)` actually results in a return-only generic. TypeScript is a little awkward about these and it will actually be typed as `Foo<unknown>`. This is unfortunate, and the best practice around return-only generics is generally considered casting them to the desired type at each call site. This means that a user should really do `ctor.new(Foo) as ctor<Foo<T>>`, which is a bit annoying syntatically, but not that big a deal in the grand scheme of things.

More annoyingly is that TypeScript does not allow type parameters in the `extends` clause of a class when used as an expression. This makes sense for TypeScript because a class type `T` is not known until the class is instantiated, long after it was declared as extending a different class. However, with the recent changes in `ctor<T>`, the inheritance hierarchy is not built until construction-time, so we actually do know the value of `T`. Unfortunately this can't be expressed in TypeScript, so our best option is to simply `@ts-ignore` the error. This is not ideal, but TypeScript *mostly* does what we want in this case. The one issue here is that superclass members are typed as `any`, which is quite unfortunate but I don't see a good way around that.

See: microsoft/TypeScript#26154 (comment)
@rzvc
Copy link

rzvc commented Feb 23, 2022

Hey guys, can we please reopen this issue? The problem still exists, and I believe the arguments made against it are wrong.

In reply to @RyanCavanaugh's answer:

Yes, test() gets called only once - but that's all you want anyway. The argument that it doesn't vary based on its type doesn't mean much, because generics in typescript don't really have any effect on the generated code. They're there only for the type system.

I'm going to go further and say that I was wrong too, when I partially agreed that the parameters are presenting a problem, because now I realize that there is a possible solution.

In the example you gave:

function test<T>(argument: T): new () => T
{
    return null;
}

class Foo<T> extends test<T>(/* provide an argument */)
{
	public constructor()
	{
		super();
	}
}

You could have the following:

class Foo<T> extends test<T>(Bar)
{
	public constructor()
	{
		super();
	}
}

What this means for Foo is that the validity of Bar is going to be checked when Foo will get declared. If Bar is a Whatever, then it passes, if not, it fails, so the arguments of the constructor signature simply place a constraint on T.

I understand that there are practical considerations here, because test() would get called regardless of the existence of any Foo instances, but if the type parameters on the constructor signature can be inferred or temporarily discarded (in the case where they're only used as a return type), then this should be theoretically implementable.

To summarize:

  1. test() gets called only once. It doesn't vary based on its template arguments anyway.
  2. The validity of test()'s parameters, can be verified independent of Foo, and they can definitely be passed before the declaration of any Foo instances, as long as their type doesn't depend on template parameters, or if it can be inferred.
  3. When Foo instances are declared, test()'s parameters become constraints on what T can be.

@ShaMan123
Copy link

At the very least I would expect that declaration merging would be possible over a declared class to workaround this limitation but it isn't.
Not being able to workaround a relatively simple case as this is really 👎

// ------------------------------------ 

type Constructor<T = object, A extends any[] = any[]> = new (...a: A) => T

function FooMixin<T extends Constructor>(Base: T) { 

    class Foo extends Base { 
        foo = 'foo'
    }

    return Foo as typeof Foo & T
}

function BarMixin<T extends Constructor>(Base: T) { 

    class Bar extends FooMixin(Base as unknown as Constructor) { 
        test() { 
            console.log(this.foo)
        }
    }

    return Bar as typeof Bar & T

}

class Base<T> {
    d(x: T){
        return 2
    }
}

// ------------------------------------  reproduction

// This is where the true bug lies!
// declaration merging throws an error so it is impossible to workaround this limitation
interface A<T extends string> extends Base<T>{

}

// T & T are unrelated so it is eq to `Base<any>`
class A<T extends string> extends FooMixin(Base<T>) {

}


class NonGenericSubClass extends BarMixin(Base<string>) {

}

new A().d(4); // any :(
console.log(new A().foo)

new NonGenericSubClass().d(4);
new NonGenericSubClass().d('4');
Output
"use strict";
// ------------------------------------ 
function FooMixin(Base) {
    class Foo extends Base {
        constructor() {
            super(...arguments);
            this.foo = 'foo';
        }
    }
    return Foo;
}
function BarMixin(Base) {
    class Bar extends FooMixin(Base) {
        test() {
            console.log(this.foo);
        }
    }
    return Bar;
}
class Base {
    d(x) {
        return 2;
    }
}
// T & T are unrelated so it is eq to `Base<any>`
class A extends FooMixin((Base)) {
}
class NonGenericSubClass extends BarMixin((Base)) {
}
new A().d(4); // any :(
console.log(new A().foo);
new NonGenericSubClass().d(4);
new NonGenericSubClass().d('4');
Compiler Options
{
  "compilerOptions": {
    "strict": true,
    "noImplicitAny": true,
    "strictNullChecks": true,
    "strictFunctionTypes": true,
    "strictPropertyInitialization": true,
    "strictBindCallApply": true,
    "noImplicitThis": true,
    "noImplicitReturns": true,
    "alwaysStrict": true,
    "esModuleInterop": true,
    "declaration": true,
    "experimentalDecorators": true,
    "emitDecoratorMetadata": true,
    "target": "ES2017",
    "jsx": "react",
    "module": "ESNext",
    "moduleResolution": "node"
  }
}

Playground Link: Provided

ShaMan123 added a commit to fabricjs/fabric.js that referenced this issue Dec 28, 2022
rajsite pushed a commit to ni/nimble that referenced this issue Apr 4, 2024
…se (#1997)

# Pull Request

## 🤨 Rationale

Resolves #1964 

## 👩‍💻 Implementation

`TableColumnTextBase` cannot simply be made generic because TypeScript
doesn't support generics and mixins together:
microsoft/TypeScript#26154.

Therefore, each concrete instance of `TableColumnTextBase` should add
its own mixins in a way that they get mixed into
`TableColumnTextBase<TColumnConfig>` rather than `TableColumn`. To avoid
code duplicate, I created a function, `mixinTextBase`, that mixes in the
appropriate mixins for `TableColumnTextBase`.

## 🧪 Testing

- Unit tests still pass
- Manually verified that the type of `this.columnInternals.columnConfig`
is correct for a column using `mixinTextBase`
- Sanity tested examples in storybook to make sure columns still behave
as expected

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [ ] I have updated the project documentation to reflect my changes or
determined no changes are needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

8 participants