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

In JS Class serialization, read the base construct signature from the static base type #41767

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Read the base construct signature from the static base type, not the …
…instance base
  • Loading branch information
weswigham committed Dec 1, 2020

Verified

This commit was signed with the committer’s verified signature.
weswigham Wesley Wigham
commit a3921800ce1eeeef1935ce826439c93f70532996
2 changes: 1 addition & 1 deletion src/compiler/checker.ts
Original file line number Diff line number Diff line change
@@ -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[];
Copy link
Contributor

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 😂

const indexSignatures = serializeIndexSignatures(classType, baseTypes[0]);
addResult(setTextRange(factory.createClassDeclaration(
/*decorators*/ undefined,
4 changes: 0 additions & 4 deletions tests/baselines/reference/jsDeclarationsClasses.js
Original file line number Diff line number Diff line change
@@ -577,10 +577,6 @@ export class N<T> extends L {
* @extends {N<U>}
*/
export class O<U> extends N<U> {
/**
* @param {U} param
*/
constructor(param: U);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

@weswigham weswigham Dec 2, 2020

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.

Copy link
Member

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.

Copy link
Member Author

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.

another2: U;
}
declare const VariableBase_base: any;
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
//// [index.js]
export class Super {
/**
* @param {string} firstArg
* @param {string} secondArg
*/
constructor(firstArg, secondArg) { }
}

export class Sub extends Super {
constructor() {
super('first', 'second');
}
}

//// [index.js]
"use strict";
var __extends = (this && this.__extends) || (function () {
var extendStatics = function (d, b) {
extendStatics = Object.setPrototypeOf ||
({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) ||
function (d, b) { for (var p in b) if (Object.prototype.hasOwnProperty.call(b, p)) d[p] = b[p]; };
return extendStatics(d, b);
};
return function (d, b) {
extendStatics(d, b);
function __() { this.constructor = d; }
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
};
})();
exports.__esModule = true;
exports.Sub = exports.Super = void 0;
var Super = /** @class */ (function () {
/**
* @param {string} firstArg
* @param {string} secondArg
*/
function Super(firstArg, secondArg) {
}
return Super;
}());
exports.Super = Super;
var Sub = /** @class */ (function (_super) {
__extends(Sub, _super);
function Sub() {
return _super.call(this, 'first', 'second') || this;
}
return Sub;
}(Super));
exports.Sub = Sub;


//// [index.d.ts]
export class Super {
/**
* @param {string} firstArg
* @param {string} secondArg
*/
constructor(firstArg: string, secondArg: string);
}
export class Sub extends Super {
constructor();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
=== tests/cases/conformance/jsdoc/declarations/index.js ===
export class Super {
>Super : Symbol(Super, Decl(index.js, 0, 0))

/**
* @param {string} firstArg
* @param {string} secondArg
*/
constructor(firstArg, secondArg) { }
>firstArg : Symbol(firstArg, Decl(index.js, 5, 16))
>secondArg : Symbol(secondArg, Decl(index.js, 5, 25))
}

export class Sub extends Super {
>Sub : Symbol(Sub, Decl(index.js, 6, 1))
>Super : Symbol(Super, Decl(index.js, 0, 0))

constructor() {
super('first', 'second');
>super : Symbol(Super, Decl(index.js, 0, 0))
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
=== tests/cases/conformance/jsdoc/declarations/index.js ===
export class Super {
>Super : Super

/**
* @param {string} firstArg
* @param {string} secondArg
*/
constructor(firstArg, secondArg) { }
>firstArg : string
>secondArg : string
}

export class Sub extends Super {
>Sub : Sub
>Super : Super

constructor() {
super('first', 'second');
>super('first', 'second') : void
>super : typeof Super
>'first' : "first"
>'second' : "second"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// @allowJs: true
// @checkJs: true
// @outDir: /out
// @lib: es6
// @declaration: true
// @filename: index.js
export class Super {
/**
* @param {string} firstArg
* @param {string} secondArg
*/
constructor(firstArg, secondArg) { }
}

export class Sub extends Super {
constructor() {
super('first', 'second');
}
}