-
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
In JS Class serialization, read the base construct signature from the static base type #41767
In JS Class serialization, read the base construct signature from the static base type #41767
Conversation
/** | ||
* @param {U} param | ||
*/ | ||
constructor(param: U); |
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.
Why is this elided? source has constructor signature for O at https://github.com/microsoft/TypeScript/pull/41767/files#diff-8cef264cf87057b69d50eeda6098081c028ab139d5c75afecc19512c8e92e824R178
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.
It's identical to the base signature (the base signature is instantiated into the same signature as in the sub class), and thus redundant.
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.
Interesting.. i thought that would be elided only if O had elided it.
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.
Nah - the node serializer looks at semantic information usually, not syntactic. Sometimes we reuse input type nodes where possible, but generally not wholesale signature declarations like this. So since the class only has one signature, and it's identical to the signature provided by the base class, we "elide" it so as to not include the base class's signature in all subclasses, since we don't need it.
@@ -6772,7 +6772,7 @@ namespace ts { | |||
!some(getSignaturesOfType(staticType, SignatureKind.Construct)); | |||
const constructors = isNonConstructableClassLikeInJsFile ? | |||
[factory.createConstructorDeclaration(/*decorators*/ undefined, factory.createModifiersFromModifierFlags(ModifierFlags.Private), [], /*body*/ undefined)] : | |||
serializeSignatures(SignatureKind.Construct, staticType, baseTypes[0], SyntaxKind.Constructor) as ConstructorDeclaration[]; | |||
serializeSignatures(SignatureKind.Construct, staticType, staticBaseType, SyntaxKind.Constructor) as ConstructorDeclaration[]; |
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.
I love the subtlety of this fix and I keep on being amazed that DevTools "fuzzes" TypeScript 😂
And not the instance base type, where non-static members come from.
Fixes #41397