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

A mixin class must have a constructor with a single rest parameter of type 'any[]'. #37142

Open
bennypowers opened this issue Mar 1, 2020 · 21 comments
Labels
Bug A bug in TypeScript
Milestone

Comments

@bennypowers
Copy link

TypeScript Version: 3.8.2

Search Terms: "A mixin class must have a constructor with a single rest parameter of type 'any[]'."

Code

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

export function DataMixin<TBase extends Constructor<{}>>(
  superclass: TBase
) {
  return class DataClass<TData> extends superclass {
    data: TData = null;
  };
}

export function QueryMixin<TBase extends Constructor<{}>>(
  superclass: TBase
) {
  return class QueryClass<TData> extends DataMixin<TBase>(superclass)<TData> {
    query(): TData {
      return this.data;
    }
  };
}

Expected behavior:

I'd expect this to compile without error

Actual behavior:

error TS2545: A mixin class must have a constructor with a single rest parameter of type 'any[]'.

Playground Link: Playground Link

Related Issues: #16390

@canonic-epicure
Copy link

canonic-epicure commented Sep 29, 2020

One more example: https://www.typescriptlang.org/play?#code/MYGwhgzhAEDKD2BbAptA3gWAFAF9vdEhgEEA7eAFwAtkAndbaJ6AIwEtSATAYXCmgA8vGMgAeFZFxgUAngAdk8AGZwkyAHzQAFMHikYALmjCAlNCPCGWZjei1kFAK61S0QvzESpbvTEzXbJjwAoOwcIA

class Some {
}
class Another {
    bindClass <Cls extends typeof Some> (cons : Cls) : Cls {
        return class extends cons {
        }
    }
}

I think the behavior A mixin class must have a constructor with a single rest parameter of type 'any[]' is incorrect.

Should be:
A mixin class must have a constructor with a single rest parameter of type 'any[]', or no constructor, or constructor w/o arguments`

@yw662
Copy link

yw662 commented Oct 28, 2020

This is actually more annoying than you think, like this:

type ConstructorOf<T extends Object, Args extends any[]> = {
    prototype: T
    new(...args: Args): T
}

function mixin<T extends Object,
    Args extends any[],
    S extends ConstructorOf<T, Args>>(Self: S) {
    return class Minxin extends Self {
        constructor(...args: Args) {
            super(...args)
        }
    }
}

This "disability" of TS actually make it unable to perform type check on parameters of mixin constructors.

@abdatta
Copy link

abdatta commented Nov 16, 2020

Hi, I'm trying to write a simple Mixin in TS that should only extend classes with no constructor parameters. Here's the code:

const BaseComponent = <T extends new () => {}>(superClass: T) => class extends superClass {
  constructor() {
    super();
  }
}

However, this gives the error: A mixin class must have a constructor with a single rest parameter of type 'any[]'
This gets resolved if I add rest parameters, like this:

const BaseComponent = <T extends new (...args: any[]) => {}>(superClass: T) => class extends superClass {
  constructor(...args: any[]) {
    super();
  }
}

But I don't want the function to accept any superClass with parameterized constructors at all. Is there a way to enforce that in Typescript?

@Aumeeb
Copy link

Aumeeb commented Jan 1, 2021

interface Printable {
    print(text: string): string
    printerVersion: string
    availablePage: number
  }
  
  class Printer implements Printable {
    constructor(public availablePage: number, public printerVersion: string) {}
    print(text: string): string {
      return 'my company printer : ' + text
    }
  }
  
  function replacePrinter<
    Base extends new (...args: ConstructorParameters<typeof Printer>) => Base
  >(base: Base): Base {
    return class extends base {
      constructor(...args: any[]) {
        super(args[0], args[1])
      }
    }
  }
  const lnkPrinter = replacePrinter(
    class {
      printerVersion
      availablePage
      constructor(printerVersion, availablePage) {
        this.printerVersion = printerVersion
        this.availablePage = availablePage
      }
      print(text: string): string {
        return `lnk printer ${text}`
      }
    }
  )
  
  const localPrinter = new Printer(200,'print')   //  new lnkPrinter(100,'🐷')
//   const localPrinter = new lnkPrinter(100, 'ink') //  new lnkPrinter(100,'🐷')
  console.log(localPrinter)
  
  console.log(localPrinter.print('zzo'))
  console.log(localPrinter.availablePage)
  console.log(localPrinter.printerVersion)
  

@Aumeeb
Copy link

Aumeeb commented Jan 1, 2021

This code above able to run, but with syntax errors.

(local class) (Anonymous class)
Class '(Anonymous class)' incorrectly extends base class 'Base'.
'(Anonymous class)' is assignable to the constraint of type 'Base', but 'Base' could be instantiated with a different subtype of constraint 'new (availablePage: number, printerVersion: string) => Base'.ts(2415)
A mixin class must have a constructor with a single rest parameter of type 'any[]'.ts(2545)

I expected #1 function replacePrinter() don't give me error
#2 The implementation after called function replacePrinter() will give me conrrent parameters within constructor(...)

@abdatta
Copy link

abdatta commented Apr 3, 2021

I'm trying to create mixins that take extra parameters in their constructors, but TS seems to disallow it.
See code:

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

const A = <T extends Constructor>(superClass: T) => class extends superClass {
    a: number;
    constructor(...args: any[]) {
        const [a, ...superArgs] = args;
        super(...superArgs);
        this.a = a;
    }
    helloA() { console.log('A:', this.a) }
}

const B = <T extends Constructor>(superClass: T) => class extends superClass {
    b: number;
    constructor(...args: any[]) {
        const [b, ...superArgs] = args;
        super(...superArgs);
        this.b = b;
    }
    helloB() { console.log('B:', this.b) }
}

class C {
    constructor(public c: number) {}
    helloC() { console.log('C:', this.c) }
}

const ABC = A(B(C));
const abc = new ABC(10, 11, 12); // tsc: Expected 1 arguments, but got 3.

There are multiple things that I don't like I have to do using TS.

  1. First, I have to type all my constructor arguments as any[], which completely makes me lose the type checking inside the constructor, and when calling super.
  2. Then, I expect ABC constructor to have any[] as the constructor argument (because that is how we defined it), but it looks like it only expects the arguments as in the constructor of C (c: number).

The ideal way to write this should be:

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

const A = <T extends Constructor>(superClass: T) => class extends superClass {
    constructor(public a: number, ...args: ConstructorParameters<T>) {
        super(...args);
    }
    helloA() { console.log('A:', this.a) }
}

const B = <T extends Constructor>(superClass: T) => class extends superClass {
    constructor(public b: number, ...args: ConstructorParameters<T>) {
        super(...args);
    }
    helloB() { console.log('B:', this.b) }
}

class C {
    constructor(public c: number) {}
    helloC() { console.log('C:', this.c) }
}

const ABC = A(B(C));
const abc = new ABC(10, 11, 12); // tsc should expect: `new ABC(a: number, b: number, c: number)`

Hi @RyanCavanaugh , can you please confirm if this can be done as above. If so, I'm willing to help out to make this possible. Some help will be appreciated as it will be my first time diving into Typescript's codebase.

@canonic-epicure
Copy link

@abdatta This is by design, since mixin is supposed to be applied to arbitrary class. When using mixins, better to create your own uniform static constructor method, like here

@abdatta
Copy link

abdatta commented Apr 4, 2021

@canonic-epicure My example above also assumes any arbitrary class for the mixins A and B. But all I'm wanting is that, the "mixined" class that is generated from A or B, should have the ability to modify the constructor parameters of the generated class, if they want so. JS doesn't have any issues with it, then why does TS force us not to write like this?

In other words, why force the any[] constructor parameter type? Won't it be fine as long as super is called with ConstructorParameters<T> of the base class generic, irrespective of whatever the parameters of the subclass constructor is? (What I mean by that, is that the following code should also be valid):

    constructor(public arg1, public arg2, ...superArgs: ConstructorParameters<T>, public arg3, public arg4) {
        super(...superArgs);
    }

Also, as a separate point, I would also like to ask why is it that "mixin is supposed to be applied to arbitrary class" only? Why can't we write a mixin that we want to be limited to only a certain type of classes?

type ClassWithDate = new (...args: any[]) => { date: Date };

const dateDisplayMixin = <T extends ClassWithDate>(superClass: T) => class extends superClass {
    constructor(...args: ConstructorParameters<T>) {
        super(...args);
    }
    displayDate() {
        console.log('Date: ' + this.date); 
    }
}

I feel this kind of limits our creativity on what we can achieve with mixins.

@yw662
Copy link

yw662 commented Apr 4, 2021

JS doesn't have any issues with it, then why does TS force us not to write like this?

I am with you on it, but the thing is, you can do the same thing with the single ...args: any[], by giving up strict type checking.
TS is not aiming to be safe and sound, the type checking of TS is more for convenience, that is why they make mixin this way.

@abdatta
Copy link

abdatta commented Apr 4, 2021

you can do the same thing with the single ...args: any[],

That's not entirely true though. If you see my snippet here: #37142 (comment), even though I used ...args: any[], TS will enforce the type of the mixin constructor to be the same as the base class' constructor. If it was ...any[] that would have still be fine, but TS simply forces it to be that of the base class' constructor.

TS is not aiming to be safe and sound, the type checking of TS is more for convenience

As a side note, I would be disappointed though if TS is not aiming for safety. One of the primary reasons I use TS in my production apps is because of the safety it provides by compiling code beforehand and reporting bugs. I thought safety was one of the keypoints of TS.

@yw662
Copy link

yw662 commented Apr 4, 2021

Then maybe you can give it another any somewhere (since it is already effectively any).
Or I would suggest make it a has-a instead of is-a, which is how I work around this problem.
For the safety (soundness), I think you may have read this one https://www.typescriptlang.org/docs/handbook/type-compatibility.html

@abdatta
Copy link

abdatta commented Apr 4, 2021

For the safety (soundness), I think you may have read this one https://www.typescriptlang.org/docs/handbook/type-compatibility.html

Thanks for the link to the doc. It's interesting to know about the unsound cases of Typescript. 💡

Or I would suggest make it a has-a instead of is-a, which is how I work around this problem.

About this, yeah that is how I have to refactor things currently and make them work. But I do not like that TS has an opinionated rule here, even though JS allows it perfectly fine, and it's not even a bad practice that we should avoid.

I'm open to working on a PR for this if the Typescript team gives it a go.

@canonic-epicure
Copy link

Any hope for at least fixing the case, when mixin has no constructor? That is definitely a valid case.

@michaeljsmith
Copy link

TS is not aiming to be safe and sound, the type checking of TS is more for convenience, that is why they make mixin this way.

The issue is that this check is actually making things less convenient :(

@Wyctus
Copy link

Wyctus commented Mar 25, 2022

What is the status of this?

@JomoPipi
Copy link

How would @ts-ignoreing this alter compilation?

@glektarssza
Copy link

glektarssza commented Sep 19, 2022

This issue continues to get increasingly annoying. Despite best practice recommendations of using unknown instead of any this error comes up if you don't use any as the constructor parameter.

The following code will error:

interface Geometry {
    vertices: vec3[];
}

type Constructor<TResult, TParams extends unknown[] = unknown[]> = new (
    ...params: TParams
) => TResult;

export function Indexed<TBase extends Constructor<Geometry>>(Base: TBase) {
    return class extends Base { // This line errors with TS2545
        constructor(...args: any[]) {
            super(...args);
        }
    };
}

where as the below won't:

interface Geometry {
    vertices: vec3[];
}

type Constructor<TResult, TParams extends any[] = any[]> = new (
    ...params: TParams
) => TResult;

export function Indexed<TBase extends Constructor<Geometry>>(Base: TBase) {
    return class extends Base {
        constructor(...args: any[]) {
            super(...args);
        }
    };
}

@quadristan
Copy link

I have a case of this happening also with generics and I found a solution for my own case

interface Class<T, TArgs extends unknown[]> {
    new(...args: TArgs): T
}

interface Service {
    test(): void;
}

// This is a service i want to mock
class BaseService implements Service {
    // i want to mock the dependencies
    constructor(private name: string) {
    }
    test() {
        console.log('Hello, i am '+this.test)
    }
}

export function mockService<T extends Class<Service, [string]>>(baseClass: T) {

    // A mixin class must have a constructor with a single rest parameter of type 'any[]'.(2545)
    // But.. i AM doing it ??
    // casting baseClass to  Class<Service, [string]> here will actually solve the issue
    return class extends baseClass {
        constructor(...rest:any[]){
            super("mocked name")
        }
        test(){
            super.test()
            console.log('Mocked method.')
        }
    }
}

const mocked = mockService(BaseService);
const service = new mocked()
service.test()

@boconnell
Copy link

boconnell commented Aug 23, 2023

Just to clarify some of this thread, there appear to be 2 related but distinct errors that surface with this message:

  1. Mixin classes that don't have type params need to have a constructor and be constrained on a constructor-function that takes (...args: any[]). You can work around this in a type-safe way by actually requiring the mixin and constraint to have a constructor of this form, although for reasons described above this is still not ideal. TS Playground
  2. Mixin classes with type parameters surface this error message and there is essentially no good workaround besides @ts-ignore / @ts-expect-error. TS Playground.

These could be the same underlying problem but the first one feels more like a suggestion and the second one feels more clearly like a bug, so I think it's worth considering them separately.

All playgrounds are TS v5.1.6

@yw662
Copy link

yw662 commented Aug 24, 2023

yes the problem is the later one.

@th0r
Copy link

th0r commented Sep 18, 2023

Are there any plans to do something with it, guys? It's really annoying

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript
Projects
None yet
Development

No branches or pull requests