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

[Bug]: Class declaration is broken #696

Closed
Lodin opened this issue Oct 2, 2023 · 6 comments · Fixed by #701
Closed

[Bug]: Class declaration is broken #696

Lodin opened this issue Oct 2, 2023 · 6 comments · Fixed by #701

Comments

@Lodin
Copy link

Lodin commented Oct 2, 2023

I found that the Class declaration is broken.

In this example:

export type Class<T, Arguments extends unknown[] = any[]> = {
  prototype: T;
  new (...arguments_: Arguments): T;
};

class A<T = unknown> {
  value: T;

  constructor(value: T) {
    this.value = value;
  }
}

interface BVal {
  field: number;
}

class B<T extends BVal = BVal> extends A<T> {

}

class C<T extends A> {
  instance: T;

  constructor(cls: Class<T>) {
    this.instance = new cls({ field: 10 });
  }
}

var a = new C(B);

the type of a is not C<B<BVal>> as I expected, but C<B<any>>, which breaks the type inference.

After some investigation I found that it was caused by using the T generic as a value for the prototype property. It is incorrect because the prototype in constructor should be the parent class, not the instance of the declaring class. That's why TypeScript fails to infer the correct type.

If I change prototype type to object, a has the correct type C<B<BVal>>.

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • The funding will be given to active contributors.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@sindresorhus
Copy link
Owner

// @ajvincent

@ajvincent
Copy link
Contributor

I'm not sure I buy this argument. There's a couple holes here, specifically around not specifying types explicitly.

var a = new C(B);

You haven't specified the type argument for C, and there's no default type there. If I make one change:

var a = new C<B<BVal>>(B);

then I get the expected result.

@ajvincent
Copy link
Contributor

Clarification: I'm not saying I'm right. I'm just saying you haven't convinced me yet.

@Lodin
Copy link
Author

Lodin commented Oct 9, 2023

I expect TS to infer the type from the argument here implicitly. And TS is capable of it: the type is correctly inferred when I replace the type of prototype to object. I guess, TS hiccups at the current definition because it doesn't expect the prototype of the constructor to be the same as the instance of the class.

@Lodin
Copy link
Author

Lodin commented Oct 9, 2023

Yeah, looks like it really depends on the type name. If I make a simple Copy wrapper that creates a new anonymous type, it suddenly starts working [playground]

type Copy<T> = {
  [P in keyof T]: T[P];
}

export type Class<T, Arguments extends unknown[] = any[]> = {
  prototype: Copy<T>;
  new (...arguments_: Arguments): T;
};

class A<T = unknown> {
  value: T;

  constructor(value: T) {
    this.value = value;
  }
}

interface BVal {
  field: number;
}

class B<T extends BVal = BVal> extends A<T> {

}

class C<T extends A> {
  instance: T;

  constructor(cls: Class<T>) {
    this.instance = new cls({ field: 10 });
  }
}

var a = new C(B);

@ajvincent
Copy link
Contributor

ajvincent commented Oct 9, 2023

That is very interesting. I remember the keyof operator didn't quite capture everything. Specifically, that it excludes new ()... which wouldn't make sense in the scenario we're talking about.

I think you've sold me at this point. Let me get back to you.

I also think Pick<T, keyof T> does the same thing.

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

Successfully merging a pull request may close this issue.

3 participants