TS is overly picky when declaring a class constructor type

[bcherny]

TypeScript Version: 3.4.0-dev.20190202

Search Terms: class, extend, constructor, any[]

Code

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

function mixin<C extends ClassConstructor>(Class: C) {
  return class extends Class {}
}

Expected behavior:

I should be able to replace ...args: any[] with ...args: unknown[], or any other signature.

Actual behavior:

Error: Type 'C' is not a constructor function type. [2507]

Playground Link: https://www.typescriptlang.org/play/index.html#src=type%20ClassConstructor%20%3D%20new(...args%3A%20unknown%5B%5D)%20%3D%3E%20%7B%7D%0D%0A%0D%0Afunction%20mixin%3CC%20extends%20ClassConstructor%3E(Class%3A%20C)%20%7B%0D%0A%20%20return%20class%20extends%20Class%20%7B%7D%0D%0A%7D

[dragomirtitian]

In the pull request the rest parameter of any[] is required:

In the following, the term mixin constructor type refers to a type that has a single construct signature with a single rest argument of type any[] and an object-like return type.

Not saying it should be but this is the expected behavior, unknown along came a long time after this PR, maybe the rules can be changed.

[RyanCavanaugh]

Agree, unknown[] should be an acceptable substitute. Accepting PRs for an easy fix.

[dragomirtitian]

@RyanCavanaugh So the issue seems ridiculously easy to fix (and I have the code ready), I was just wondering how to go about the tests. Should I add a new one or change one to include a version with unknown[] (this seems like a good candidate conformance/classes/mixinClassesAnnotated.ts)

[RyanCavanaugh]

@dragomirtitian no preference; whichever's easiest for you

[dragomirtitian]

@RyanCavanaugh I added a fix, just one problem (which unfortunately probably makes my fix almost useless).

The constraint new(...args: unknown[]) => {} works well as long as strictFunctionTypes are not turned on. When that flag is on this code becomes an error

class Base {
    constructor(public x: number, public y: number) {}
}
let c : new (...args: unknown[]) => {} = Base // error under strictFunctionTypes: true
new c("") // runtime type mismatch 

This means that you can't really call the mixin with anything but an empty constructor or one that has unknown arguments.

[RyanCavanaugh]

That's why it's a constraint (not a concrete type) though, right?

[dragomirtitian]

@RyanCavanaugh I can look into that, but this problem seems much bigger than mixins and would have implications broadly. Rest parameters of type unknown[] are generally not compatible with arbitrary parameter types even if we are talking about a constraint. This currently fails under strictFunctionTypes

function test<T extends (...a: unknown[]) => unknown>(fn:T) : T{
      fn(""); // this passes "" can be assigned to unknown 
      return fn;
}
test(function (a: number) {   // error here under strictFunctionTypes
})

I'm not 100% sure it is necessarily a great idea to make the code above error free . It seems to be weaker as far as type safety goes. As we see in the code above the callback passed in accepts a number but it's called inside test with a string. I think the current behavior is better, don't let a function with a number parameter be assigned to a function with an unknown parameter, the implementing function can't handle unknown.

While this is no worse than if we use any[], unknown[] would give a false sense of type-safety when no safety actually exists. Except for obeying the no-any tslint rule not sure why unknown is better in this instance. At least any would function as a '💀💀 careful unsafe 💀💀' warning, at least that's the way I see any :)