bublejs / buble

https://buble.surge.sh
MIT License
871 stars 67 forks source link

Extending Error is flaky because `Error.call` returns a new object #40

Open marijnh opened 6 years ago

marijnh commented 6 years ago

I.e. let x = {}; Error.call(x) == x produces false, at least on Firefox and Chrome. This is preventing Error subclasses compiled with Bublé from getting stack and message properties, making them rather useless.

Babel works around this by creating a new binding to replace this inside the constructor, storing the result of the super constructor call in that, and returning it at the end. Would that be something that Bublé would be interested in adding? I might be able to create a PR.

marijnh commented 6 years ago

Oh, even with that fix, the resulting instances are not instanceof your class. Even Babel doesn't get that right.

It does work when you directly use class syntax in the browser, so this is a case where compiling down does some damage.

Rich-Harris commented 6 years ago

Two possible fixes — don't transpile classes (#43), or don't transpile anything (#42). Depends whether we want to continue supporting Node 4. Any thoughts?

marijnh commented 6 years ago

Ah, I wasn't even talking about this causing a problem in the Bublé sources (I hadn't noticed)—rather it was causing trouble in my own projects. But I've fallen back on directly writing the horrible ES5 code needed to create a subclass of Error for now, and I guess I'll just keep doing that until the next geological era when I can ship client-side ES6 code. So this isn't blocking me.

adrianheine commented 6 years ago

As discussed in the babel issue, whether extending builtins is allowed should probably be configurable, right?

adrianheine commented 6 years ago

As @andarist pointed out in #93, CompileError already sets message and stack, so this should not be a problem for bublé itself.