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

Bind assignments to 'this' within static blocks in JS files #46472

Merged
merged 3 commits into from
Oct 22, 2021
Merged
Show file tree
Hide file tree
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
1 change: 1 addition & 0 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2947,6 +2947,7 @@ namespace ts {
case SyntaxKind.MethodDeclaration:
case SyntaxKind.GetAccessor:
case SyntaxKind.SetAccessor:
case SyntaxKind.ClassStaticBlockDeclaration:
// this.foo assignment in a JavaScript class
// Bind this property to the containing class
const containingClass = thisContainer.parent;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/src/a.js(10,7): error TS2417: Class static side 'typeof ElementsArray' incorrectly extends base class static side '{ isArray(arg: any): arg is any[]; readonly prototype: any[]; }'.
Types of property 'isArray' are incompatible.
Type '(arg: any) => boolean' is not assignable to type '(arg: any) => arg is any[]'.
Signature '(arg: any): boolean' must be a type predicate.


==== /src/a.js (1 errors) ====
class Thing {
static {
this.doSomething = () => {};
}
}

Thing.doSomething();

// GH#46468
class ElementsArray extends Array {
~~~~~~~~~~~~~
!!! error TS2417: Class static side 'typeof ElementsArray' incorrectly extends base class static side '{ isArray(arg: any): arg is any[]; readonly prototype: any[]; }'.
!!! error TS2417: Types of property 'isArray' are incompatible.
!!! error TS2417: Type '(arg: any) => boolean' is not assignable to type '(arg: any) => arg is any[]'.
Copy link
Member

Choose a reason for hiding this comment

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

not related to this PR, but I'm curious as to why this.isArray ends up with type (arg: any) => boolean instead of (arg: any) => arg is any[]

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't try to infer type predicate signatures - users always have to write them out deliberately, and can't rely on contextual types.

But even if we did rely on contextual types, we have another issue with class members not using them #23911 😢

!!! error TS2417: Signature '(arg: any): boolean' must be a type predicate.
static {
const superisArray = super.isArray;
const customIsArray = (arg)=> superisArray(arg);
this.isArray = customIsArray;
}
}

ElementsArray.isArray(new ElementsArray());
73 changes: 73 additions & 0 deletions tests/baselines/reference/javascriptThisAssignmentInStaticBlock.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
//// [a.js]
class Thing {
static {
this.doSomething = () => {};
}
}

Thing.doSomething();

// GH#46468
class ElementsArray extends Array {
static {
const superisArray = super.isArray;
const customIsArray = (arg)=> superisArray(arg);
this.isArray = customIsArray;
}
}

ElementsArray.isArray(new ElementsArray());

//// [a.js]
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) {
if (typeof b !== "function" && b !== null)
throw new TypeError("Class extends value " + String(b) + " is not a constructor or null");
extendStatics(d, b);
function __() { this.constructor = d; }
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
};
})();
var _a, _b;
var _this = this;
var Thing = /** @class */ (function () {
function Thing() {
}
return Thing;
}());
_a = Thing;
(function () {
_a.doSomething = function () { };
})();
Thing.doSomething();
// GH#46468
var ElementsArray = /** @class */ (function (_super) {
__extends(ElementsArray, _super);
function ElementsArray() {
return _super !== null && _super.apply(this, arguments) || this;
}
return ElementsArray;
}(Array));
_b = ElementsArray;
(function () {
var superisArray = _super.isArray;
var customIsArray = function (arg) { return superisArray(arg); };
_b.isArray = customIsArray;
})();
ElementsArray.isArray(new ElementsArray());


//// [a.d.ts]
declare class Thing {
}
Comment on lines +66 to +68
Copy link
Member Author

Choose a reason for hiding this comment

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

This is wrong, but not something I would consider in scope for this PR. I will create a follow-up issue for doSomething getting dropped in the .d.ts output.

declare class ElementsArray extends Array<any> {
constructor(arrayLength?: number);
constructor(arrayLength: number);
constructor(...items: any[]);
Comment on lines +70 to +72
Copy link
Member Author

Choose a reason for hiding this comment

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

Honestly, I don't know why this needs to be duplicated in the .d.ts emit.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
=== /src/a.js ===
class Thing {
>Thing : Symbol(Thing, Decl(a.js, 0, 0))

static {
this.doSomething = () => {};
>this.doSomething : Symbol(Thing.doSomething, Decl(a.js, 1, 12))
>this : Symbol(Thing, Decl(a.js, 0, 0))
>doSomething : Symbol(Thing.doSomething, Decl(a.js, 1, 12))
}
}

Thing.doSomething();
>Thing.doSomething : Symbol(Thing.doSomething, Decl(a.js, 1, 12))
>Thing : Symbol(Thing, Decl(a.js, 0, 0))
>doSomething : Symbol(Thing.doSomething, Decl(a.js, 1, 12))

// GH#46468
class ElementsArray extends Array {
>ElementsArray : Symbol(ElementsArray, Decl(a.js, 6, 20))
>Array : Symbol(Array, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))

static {
const superisArray = super.isArray;
>superisArray : Symbol(superisArray, Decl(a.js, 11, 13))
>super.isArray : Symbol(ArrayConstructor.isArray, Decl(lib.es5.d.ts, --, --))
>super : Symbol(ArrayConstructor, Decl(lib.es5.d.ts, --, --))
>isArray : Symbol(ArrayConstructor.isArray, Decl(lib.es5.d.ts, --, --))

const customIsArray = (arg)=> superisArray(arg);
>customIsArray : Symbol(customIsArray, Decl(a.js, 12, 13))
>arg : Symbol(arg, Decl(a.js, 12, 31))
>superisArray : Symbol(superisArray, Decl(a.js, 11, 13))
>arg : Symbol(arg, Decl(a.js, 12, 31))

this.isArray = customIsArray;
>this.isArray : Symbol(ElementsArray.isArray, Decl(a.js, 12, 56))
>this : Symbol(ElementsArray, Decl(a.js, 6, 20))
>isArray : Symbol(ElementsArray.isArray, Decl(a.js, 12, 56))
>customIsArray : Symbol(customIsArray, Decl(a.js, 12, 13))
}
}

ElementsArray.isArray(new ElementsArray());
>ElementsArray.isArray : Symbol(ElementsArray.isArray, Decl(a.js, 12, 56))
>ElementsArray : Symbol(ElementsArray, Decl(a.js, 6, 20))
>isArray : Symbol(ElementsArray.isArray, Decl(a.js, 12, 56))
>ElementsArray : Symbol(ElementsArray, Decl(a.js, 6, 20))

Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
=== /src/a.js ===
class Thing {
>Thing : Thing

static {
this.doSomething = () => {};
>this.doSomething = () => {} : () => void
>this.doSomething : () => void
>this : typeof Thing
>doSomething : () => void
>() => {} : () => void
}
}

Thing.doSomething();
>Thing.doSomething() : void
>Thing.doSomething : () => void
>Thing : typeof Thing
>doSomething : () => void

// GH#46468
class ElementsArray extends Array {
>ElementsArray : ElementsArray
>Array : any[]

static {
const superisArray = super.isArray;
>superisArray : (arg: any) => arg is any[]
>super.isArray : (arg: any) => arg is any[]
>super : ArrayConstructor
>isArray : (arg: any) => arg is any[]

const customIsArray = (arg)=> superisArray(arg);
>customIsArray : (arg: any) => boolean
>(arg)=> superisArray(arg) : (arg: any) => boolean
>arg : any
>superisArray(arg) : boolean
>superisArray : (arg: any) => arg is any[]
>arg : any

this.isArray = customIsArray;
>this.isArray = customIsArray : (arg: any) => boolean
>this.isArray : (arg: any) => boolean
>this : typeof ElementsArray
>isArray : (arg: any) => boolean
>customIsArray : (arg: any) => boolean
}
}

ElementsArray.isArray(new ElementsArray());
>ElementsArray.isArray(new ElementsArray()) : boolean
>ElementsArray.isArray : (arg: any) => boolean
>ElementsArray : typeof ElementsArray
>isArray : (arg: any) => boolean
>new ElementsArray() : ElementsArray
>ElementsArray : typeof ElementsArray

24 changes: 24 additions & 0 deletions tests/cases/compiler/javascriptThisAssignmentInStaticBlock.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// @allowJs: true
// @checkJs: true
// @declaration: true
// @outDir: /out/
// @filename: /src/a.js

class Thing {
static {
this.doSomething = () => {};
}
}

Thing.doSomething();

// GH#46468
class ElementsArray extends Array {
static {
const superisArray = super.isArray;
const customIsArray = (arg)=> superisArray(arg);
this.isArray = customIsArray;
}
}

ElementsArray.isArray(new ElementsArray());