cramforce/splittable

Incorrectly transpiles classes

matthewp opened this issue · 4 comments

matthewp commented

This input code:

class MyWidget extends HTMLElement {
  connectedCallback(){
    console.log("hello world");
  }
}

window['customElements']['define']('my-widget', MyWidget);

Compiles to:

function f(a, b) {
    function d() {}
    d.prototype = b.prototype;
    a.prototype = new d;
    a.prototype.constructor = a;
    for (var c in b)
        if (Object.defineProperties) {
            var e = Object.getOwnPropertyDescriptor(b, c);
            e && Object.defineProperty(a, c, e)
        } else
            a[c] = b[c]
}
function g(a) {
    HTMLElement.apply(this, arguments)
}
f(g, HTMLElement);
window.customElements.define("my-widget", g);

Note the HTMLElement.apply; you can't do that. It should instead transpile to: return Reflect.construct(HTMLElement, arguments, this.constructor). I think this still wouldn't work without a shim though.

Ideally there would be a way to turn off -> ES5 transpiling though, as it is more verbose than the ES6 equivalent. Also, in this case by using Reflect.construct you would be doubling the number of objects that get created. The ES class version is just better.

Maybe I should post an issue in Closure's issue tracker for this though?

cramforce commented

Yeah, nothing splittable can do about it. Closure certainly favors fast
over correct, although that transpile is pretty suspicious for various
reasons.

On Nov 17, 2016 9:37 AM, "Matthew Phillips" notifications@github.com
wrote:

This input code:

class MyWidget extends HTMLElement {
connectedCallback(){
console.log("hello world");
}
}
window['customElements']['define']('my-widget', MyWidget);

Compiles to:

function f(a, b) {
function d() {}
d.prototype = b.prototype;
a.prototype = new d;
a.prototype.constructor = a;
for (var c in b)
if (Object.defineProperties) {
var e = Object.getOwnPropertyDescriptor(b, c);
e && Object.defineProperty(a, c, e)
} else
a[c] = b[c]
}function g(a) {
HTMLElement.apply(this, arguments)
}f(g, HTMLElement);window.customElements.define("my-widget", g);

Note the HTMLElement.apply; you can't do that. It should instead
transpile to: return Reflect.construct(HTMLElement, arguments,
this.constructor). I think this still wouldn't work without a shim though.

Ideally there would be a way to turn off -> ES5 transpiling though, as
it is more verbose than the ES6 equivalent. Also, in this case by using
Reflect.construct you would be doubling the number of objects that get
created. The ES class version is just better.

Maybe I should post an issue in Closure's issue tracker for this though?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#8, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAFeT8qp7c7P7Po-63VghWvvUUCprBKHks5q_JDcgaJpZM4K1mDs
.

matthewp commented

I'm not sure why this code would be faster than the correct ES2015 version though. Even if you replaced the HTMLElement.apply (which throws currently) with a Reflect.construct call it would be slower.

cramforce commented

Lets move to Closure Compiler github https://github.com/google/closure-compiler