-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
No compile error thrown when this
referenced before call to super
completes
#8060
Comments
This code isn't necessarily invalid -- it's entirely possible (i.e. we can't see its implementation) that the super constructor doesn't invoke the method immediately, in which case |
@RyanCavanaugh the ES6 code: class A {
constructor(fn) {
fn.call(this);
this.foo = 'foo';
}
}
class B extends A {
constructor() {
super(() => {
console.log(this);
});
this.bar = 'bar';
}
}
const b = new B(); Fails in Chrome with:
And in Edge:
And Firefox:
It is because it is a lambda function and therefore it is trying to access the lexical this, which isn't available until after |
Hmmm... wow, ok, I can break it again if I call it within the constructor: class A {
constructor(fn) {
this.foo = 'foo';
this.print = fn;
fn();
}
}
class B extends A {
constructor() {
super(() => {
console.log(this.bar);
});
this.bar = 'bar';
}
}
const b = new B();
console.log(b.foo);
b.print(); The specific use case that @tomdye and I ran into was a situation where we were extending But that is really difficult to guard against, I suspect. |
@bryanforbes just suggest that given: class B extends A {
constructor() {
super(() => {
console.log(this);
});
}
bar: string = 'bar';
} If the emit was: var B = (function (_super) {
__extends(B, _super);
function B() {
var _this;
_super.call(this, function () {
console.log(_this);
});
_this = this;
this.bar = 'bar';
}
return B;
}(A)); Then the runtime behaviour of ES5 would match ES6. |
That seems like a good solution, though I'm slightly afraid of breaking existing working code. |
It will only break code that's invalid ES6 in the first place though. If they were ever to try setting the compile target to 'es6' they would see this error at runtime. Must be better to catch it at compile time now rather than down the line should they choose to compile to 'es6' at a later date. |
The problem is we won't be catching at compile time -- people will upgrade to the next TypeScript version, recompile to ES5, and their code will start breaking at runtime with some very confusing errors and emit that looks like a bug. |
Yes, agreed. It is clearly a breaking change, but one that preserves the runtime semantics. Maybe there is room for a linter to flag uses of lexical |
Accepting PRs once the new transform-based emitter merges. Warning, this is likely a difficult change. |
Just a note on this, the emit in master (TS 2.1) now breaks these examples in ES5 at run-time, so this now has parity. It still does not warn you that it is invalid. Given: class A {
constructor(fn: () => void) {
fn.call(this);
}
foo: string = 'foo';
}
class B extends A {
constructor() {
super(() => {
console.log(this);
});
}
bar: string = 'bar';
} The emit is... Version 2.1.0-dev.20161103 var __extends = (this && this.__extends) || function (d, b) {
for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
function __() { this.constructor = d; }
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
};
var A = (function () {
function A(fn) {
this.foo = 'foo';
fn.call(this);
}
return A;
}());
var B = (function (_super) {
__extends(B, _super);
function B() {
var _this = _super.call(this, function () {
console.log(_this);
}) || this;
_this.bar = 'bar';
return _this;
}
return B;
}(A)); Version 2.0.6 var __extends = (this && this.__extends) || function (d, b) {
for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
function __() { this.constructor = d; }
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
};
var A = (function () {
function A(fn) {
this.foo = 'foo';
fn.call(this);
}
return A;
}());
var B = (function (_super) {
__extends(B, _super);
function B() {
var _this = this;
_super.call(this, function () {
console.log(_this);
});
this.bar = 'bar';
}
return B;
}(A)); |
One more case (https://gist.github.com/zemlanin/a306e8498d05c923cac405c047014029): // this.ts
class A {
constructor(protected cb: () => void) { }
}
class Greeter extends A {
constructor() {
// ^ there should be `var _this = this;`
super(() => { this })
}
} // compiled_with_ts2_0_10.js
var __extends = (this && this.__extends) || function (d, b) {
for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
function __() { this.constructor = d; }
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
};
var A = (function () {
function A(cb) {
this.cb = cb;
}
return A;
}());
var Greeter = (function (_super) {
__extends(Greeter, _super);
function Greeter() {
var _this = this;
// ^ there should be `var _this = this;`
_super.call(this, function () { _this; });
}
return Greeter;
}(A)); // compiled_with_ts2_1_1.js
var __extends = (this && this.__extends) || function (d, b) {
for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
function __() { this.constructor = d; }
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
};
var A = (function () {
function A(cb) {
this.cb = cb;
}
return A;
}());
var Greeter = (function (_super) {
__extends(Greeter, _super);
function Greeter() {
return
// ^ there should be `var _this = this;`
_super.call(this, function () { _this; }) || this;
}
return Greeter;
}(A)); |
closes #312 bump typescript version to 2.1.1 update node flags check in AstUtils accept `JsxOpeningLikeElement` skip OmittedExpressions in prefer-const accept `NewExpression` alongside with `CallExpression` correct `validateUnaryExpression` signature ES6 => ES2015 visit ts.moduleBlock skip omitted expressions skip string literal in JSX elements don’t check left and right operands on *unary* operator make test-data compilable fix `this` referencing microsoft/TypeScript#8060 fix AstUtils.isExported In rev. 9385bba , I’ve assumed that `Export` flag was moved to `ModifiedFlags`. In fact, it was renamed to `ExportContext` custom elements without <img /> in them update line positions fixing linter closes #312
I've also found this with creating an arrow function before calling super. class Foo extends Object {
public constructor() {
const test = () => {
console.log(this);
};
test();
super();
}
} You can see here that we the output has |
TypeScript Version:
1.8.9
Code
JSFiddle: https://jsfiddle.net/kitsonk/fs9t96ep/
TS Playground: http://goo.gl/X7cgvV
Expected behavior:
Typescript should guard against the use of
this
as the call tosuper
has not completed. Thus it should not compile.Actual behavior:
Typescript compiles this successfully. It works fine when the target is set to
es5
but breaks the browser when target is set toes6
.Error is: VM89:55 Uncaught ReferenceError: this is not defined
The text was updated successfully, but these errors were encountered: