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

Cannot inherit Error #12581

Closed
acrazing opened this issue Nov 30, 2016 · 8 comments
Closed

Cannot inherit Error #12581

acrazing opened this issue Nov 30, 2016 · 8 comments
Labels
Duplicate An existing issue was already created

Comments

@acrazing
Copy link

The test code as follow:

class A {}

class B extends A {}

class C extends Error {}

console.log(new B instanceof B, new B instanceof A, new B instanceof Object)

console.log(new C instanceof C, new C instanceof Error)

The compile result Between typescript version 2.0.6 and 2.1.1 is different:

The 2.0.6 emit code as follow:

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() {
    }
    return A;
}());
var B = (function (_super) {
    __extends(B, _super);
    function B() {
        _super.apply(this, arguments);
    }
    return B;
}(A));
var C = (function (_super) {
    __extends(C, _super);
    function C() {
        _super.apply(this, arguments);
    }
    return C;
}(Error));
console.log(new B instanceof B, new B instanceof A, new B instanceof Object);
console.log(new C instanceof C, new C instanceof Error);

And the 2.1.1 version emits as follow:

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() {
    }
    return A;
}());
var B = (function (_super) {
    __extends(B, _super);
    function B() {
        return _super.apply(this, arguments) || this;
    }
    return B;
}(A));
var C = (function (_super) {
    __extends(C, _super);
    function C() {
        return _super.apply(this, arguments) || this;
    }
    return C;
}(Error));
console.log(new B instanceof B, new B instanceof A, new B instanceof Object);
console.log(new C instanceof C, new C instanceof Error);

The execute result is different:

# typescript version 2.0.6 emitted code output:
true true true
true true

# typescript version 2.1.1 emitted code output:
true true true
false true
@aluanhaddad
Copy link
Contributor

https://goo.gl/photos/xysW5TXeXyaWtVPm8

@acrazing
Copy link
Author

acrazing commented Dec 1, 2016

@aluanhaddad
I want to know why add return this in a constructor function?

In es5, if a function is called as a constructor, just like new Xxx(), it should not return any thing.

@aluanhaddad
Copy link
Contributor

In es5, if a function is called as a constructor, just like new Xxx(), it should not return any thing.

That is not true.

I want to know why add return this in a constructor function?

Please read one of those dozens of issues. It discussed at length.

@HerringtonDarkholme
Copy link
Contributor

#10166

@kayahr
Copy link

kayahr commented Dec 6, 2016

Shouldn't this bug be marked as a blocker for the 2.1 milestone? Looks like a serious issue to me. My projects are completely broken when switching to 2.1 because instanceof checks in error handling no longer works.

class MyError extends Error {
    public constructor(message?: string) {
        super(message);       
    }
}

try {
    throw new MyError("Custom error");
} catch (e) {
    if (e instanceof MyError) {
        // Handle custom error   <-- TypeScript 2.0 correctly ends up here
    } else {
        throw e; // <-- TypeScript 2.1 and 2.2 ends up here
    }
}

So now all custom errors are only instance of Error. The whole class hierarchy is lost and can not be checked anymore. myError.constructor also returns the wrong constructor. Should return MyError but returns Error.

On the other hand it's nice that no special magic is needed any longer in the constructor to get error message and stack trace working in the custom error but breaking instanceof checks is a no-go.

When removing the typescript-specific markup in the code and running it in a browser directly then the browser also correctly ends up in the instanceof block.

So TypeScript 2.1 and newer compiles correct ES6 code into broken ES5 code. In TypeScript 2.0 this was working fine.

Maybe instead of fouling around with the wrong this pointer in the constructor you should add some special constructor handling when extending Error. This is what I used in TypeScript 2.0 to get correct error messages and useful stack traces:

class MyError extends Error {
    public name: string;
    public stack: string;
    public constructor(message?: string) {
        const error = super(message);
        this.message = error.message;
        this.stack = error.stack;
    }
}

@acrazing
Copy link
Author

acrazing commented Dec 6, 2016

@kayahr Me too! And it wasted my whole day.

I think this is a compromising solution for losing properties.

If you write code as your style, why not remove extends Error, it's useless? Just like follow:

class MyError {
  readonly name = 'MyError'
  readonly stack: string
  readonly message: string
  constructor(message?: string) {
    const error = new Error(message)
    this.stack  = error.stack
    this.message = error.message
  }
}

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Dec 6, 2016

The very fact that you have been having to perform these hacks just to capture the message and stack is an indication that the pattern is erroneous.

These custom error classes were not inheriting from their specified parent class in even the most fundamental sense of the notion. They were just glorified tags that required not insignificant amounts of boilerplate, non idiomatic code to consume.

Workarounds are available, but I'm just curious why a factory is insufficient? The factory can set up the necessary constructor and prototype links to minimize client breakage while you gradually move to a more reliable API.

instanceof checks in a language where the lineage of objects is mutable are always going to be weak. In this case they were not only weak but vacuous.

@DanielRosenwasser
Copy link
Member

I like @aluanhaddad's suggestion of using a factory function of some sort.

Otherwise, I'm marking this as a duplicate of #12123. This is an intentional breaking change and documented here. Sorry about the inconveniences you may have encountered.

@DanielRosenwasser DanielRosenwasser added the Duplicate An existing issue was already created label Dec 6, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

5 participants