-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Support 'typeof class' types #41587
base: main
Are you sure you want to change the base?
Support 'typeof class' types #41587
Conversation
@ahejlsberg How will type parameters interact with this feature? 👀 Is the following valid? type FixedBox<T> = typeof class {
constructor(readonly value: T);
};
declare const StringBox: FixedBox<string>; If so, am I correct in assuming that Also, is the following valid? type GenericBox = typeof class <T> {
constructor(readonly value: T);
};
declare const Box: GenericBox; In the (IMO unlikely) event that it is, would |
Random additional question: Why is // Declaration-only; no method bodies allowed
type Cat = class {
constructor(name: string);
meow(): void;
}; @rbuckton has explored a very similar syntax previously in #36392 (comment) (see the last three lines of the example). |
@treybrisbane what's nice is that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we consider changing how we manufacture types for declaration emit?
Given:
function f() {
return class {};
}
We currently emit this for the declarations:
declare function f(): {
new (): {}
};
But it seems a better option to emit this instead:
declare function f(): typeof class {};
This would be especially beneficial for more complex class definitions.
src/compiler/checker.ts
Outdated
@@ -38978,7 +38980,8 @@ namespace ts { | |||
if (flags & ModifierFlags.Abstract) { | |||
return grammarErrorOnNode(modifier, Diagnostics._0_modifier_already_seen, "abstract"); | |||
} | |||
if (node.kind !== SyntaxKind.ClassDeclaration) { | |||
// An abstract modifier is permitted on a class expression in a 'typeof abstract class {}' type | |||
if (node.kind !== SyntaxKind.ClassDeclaration && node.kind !== SyntaxKind.ClassExpression) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't that also make it valid for a regular class expression? If not, could we consider allowing it in parse? It's never made sense to me that you can write this:
function f() {
abstract class C {}
return C;
}
But not this:
function f() {
return abstract class C {};
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We certainly could allow it, but it would in effect be an expression syntax extension and we generally try to avoid those.
src/compiler/parser.ts
Outdated
@@ -2860,10 +2860,24 @@ namespace ts { | |||
return type; | |||
} | |||
|
|||
function isStartOfTypeofClassExpression() { | |||
return token() === SyntaxKind.ClassKeyword || | |||
token() === SyntaxKind.AbstractKeyword && lookAhead(() => nextToken() === SyntaxKind.ClassKeyword && !scanner.hasPrecedingLineBreak()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I mention in my comment in checker.ts
, is there a reason we don't permit abstract
for class expressions? It seems like we should always do this, not just for typeof
.
@treybrisbane Yes, both your Look for the declarations of |
The const C = class {};
type T = typeof C; can be shortened to just type T = typeof class {}; by simple substitution. If the |
Yes, but of course it brings about the classic compatibility dilemma, i.e. you wouldn't be able to generate .d.ts files with 4.2 that can be consumed by pre-4.2 versions of the compiler. |
Adds support for typeof class as part of microsoft/TypeScript#41587
} | ||
} | ||
|
||
declare function Printable2<T extends new (...args: any[]) => object>(Base: T): typeof class extends Base { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a difference between typeof class extends Base {
and typeof class extends T {
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sorry, T
is the constructor type, Base
is the instance type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be clear: T
is the constructor type, Base
is the constructor value.
Any chance we could get a playground for this? Would love to do some testing. 🙂 |
@typescript-bot pack this |
I am extremely, extremely excited about this PR 🚀 |
Soon after this PR lands, I hope there's discussion of syntax sugar for mixins/traits, similar to how it's done in Dart using the |
@matthewadams the TypeScript team doesn't want to create new non-type syntax, but I do have a proposal for mixins in JavaScript: https://github.com/justinfagnani/proposal-mixins I haven't been able to work on it in a very long time, so if there are any potential co-champions out there, let me know. |
@justinfagnani I'd consider myself to be a co-champion, and I know a couple of others that might be interested, too. I'm at matthew@matthewadams.me if you'd like to continue discussion. |
Since Orta's attempt to generate a playground for this failed, I tried testing this branch locally by installing it via Attempting to compile type BoxClass = typeof class {
constructor(readonly value: unknown);
};
const BoxClass: BoxClass = class {
constructor(readonly value: unknown) {}
}; results in
So it looks like the compiler isn't recognising the new What am I doing wrong? 😅 |
Any hope this PR can be merged any time soon? It seems to be the key piece for solving the well-known limitation of the declaration file generation (which is incompatible with class expressions). |
@@ -157,7 +157,7 @@ namespace ts.refactor { | |||
return true; | |||
} | |||
} | |||
else if (isTypeQueryNode(node)) { | |||
else if (isTypeQueryNode(node) && node.exprName.kind !== SyntaxKind.ClassExpression) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering out loud: it might be useful to make an isTypeQueryableNode
helper or similar, since this is already used twice internally, and it'd be helpful for consumers?
Pinging ... @DanielRosenwasser? - Is there anything blocking this PR beyond responding to review comments & resolving merge conflicts? +1 to the general enthusiasm for the feature landing. I'd be keen to contribute if it means this could land sooner. cc @gr2m, who pointed me here as this would help a ton with the Oktokit APIs. |
Basically a lot of unsolved questions around nominality and compatibility if you check the notes at #41824. Plus, what a lot of people really want something that allows you to encode everything from JS/TS expression land into TS declaration land, especially without generating intermediate local types, and this feature doesn't entirely do that. |
With this PR we permit type queries (the
typeof
operator in a type position) to specify class expressions. For example:Effectively,
typeof class {...}
obtains the type of the contained class expression, i.e. the type of the class constructor function object produced by the class expression. The declaration ofTC1
above is roughly equivalent tobut such declarations do not permit
protected
,private
, and/orabstract
members to be reflected. Withtypeof class
that becomes possible.Fixes #41581.