Native extends breaks HTMLELement, Array, and others

[WebReflection]

Current Babel transform, when it comes to call the parent

function _possibleConstructorReturn(self, call) {
  if (!self) {
    throw new ReferenceError("this hasn't been initialised - super() hasn't been called");
  }
  return call && (typeof call === "object" || typeof call === "function") ? call : self;
}

It's a way too poor implementation.

If we take the current basic ES6 compat syntax:

class List extends Array {}

We'll realize babel does a bad job.

var l = new List();
l instanceof List; // false

The reason is simple: Babel replace the returns and exit, without caring about userland expectations.

This is how above basic extend should desugar:

function List() {
  return Object.setPrototypeOf(
    Array.apply(this, arguments),
    List.prototype
  );
}

It's a very ad-hoc case for the initial example that currently fails, but it's good enough to understand that inheriting the prototype is the least of the problem.

Indeed, we have 3 ways to do that within a transpiled code:

// losing the initial prototype
// for polyfilled ES5+ or lower
List.prototype = Object.create(
  Array.prototype,
  {constructor: {
    configurable: true,
    writable: true,
    value: List.prototype
  }}
);

// using a cleaner way
// for ES5+ compliant targets
Object.setPrototypeOf(
  List.prototype,
  Array.prototype
);

// using a cleaner way
// that works well with
// partially polyfilled .setPrototypeOf
List.prototype = Object.setPrototypeOf(
  List.prototype,
  Array.prototype
);

Solved the inheritance bit, considering Babel also set the prototype of each constructor,
we need to address cases where a super call might "upgrade" the current context, like it is for HTMLELement or exotic native objects.

// native ES6 static example
class List extends Array {
  constructor(a, b, c) {
    super(a, b);
    this.push(c);
  }
}

Above case should desugar to something like the follwoing:

function List(a, b, c) {
  // the super bit
  var self = Object.setPrototypeOf(
    Array.call(this, a, b),
    List.prototype
  );
  // the rest with swapped context
  self.push(c);
  // at the end
  return self;
}

which is also ad-hoc example code for the previous example.

Considering a transpiler will get the arguments part easily right, this is how previous case could be generically transpiled for arguments used in both constructors:

// to make it as generic as possible
function Child() {

  // the super call bit desugar to ...
  var
    instance = Object.getPrototypeOf(Child)
        .apply(this, arguments),
    type = instance ? typeof instance : '',
    // if Parent overwrote its default return
    self = (type === 'object' || type === 'function') ?
      // upgrade the instance to reflect Child constructor
      Object.setPrototypeOf(instance, Child.prototype) :
      // otherwise use the current context as is
      this
  ;

  // at this point, the rest of the constructor
  // should use `self` instead of `this`
  self.push(c);

  // and return `self` reference at the end
  return self;
}

The last problem is that modern syntax would use Reflect to create any sort of object, instead of old, ES3 friendly, .call or .apply way.

Following a past, present, and even future proof approach:

var
  reConstruct = typeof Reflect === 'object' ?
    Reflect.construct :
    function (Parent, args, Child) {
      return Parent.apply(this, args);
    }
;

function Child() {

  // the super call bit
  var
    instance = reConstruct.call(
      this,
      Object.getPrototypeOf(Child),
      arguments,
      Child
    ),
    type = instance ? typeof instance : '',
    self = (type === 'object' || type === 'function') ?
      Object.setPrototypeOf(instance, Child.prototype) :
      this
  ;

  // the rest of the constructor body
  self.push(c);

  // at the end or instead of empty returns
  return self;
}

Above solution would work with userland, exotic, or DOM constructors, and in both native and transpiled engines.

[zloirock]

reConstruct

Without Reflect.construct:

class Class extends Date {};
+new Class(); // => ?

var
    instance = reConstruct.call(
      this,
      Object.getPrototypeOf(Child),
      arguments,
      Child
    ),
    type = instance ? typeof instance : '',
    self = (type === 'object' || type === 'function') ?
      Object.setPrototypeOf(instance, Child.prototype) :
      this
  ;

class A {
    constructor(){
        console.log(this.constructor.name);
    }
}
class B extends A {
    constructor(){
        super();
        console.log(this.constructor.name);
    }
}
class C extends B {
    constructor(){
        super();
        console.log(this.constructor.name);
    }
}
new C; // => ?

[zloirock]

BTW do you know about https://github.com/loganfsmyth/babel-plugin-transform-builtin-extend?

[WebReflection]

Right, so ... I've written probably a very poor version of the non Reflect.construct fallback.

Let me try again with an already (manually) transpiled code

function reConstruct(Parent, args, Child) {
  var boundArgs = [this];
  boundArgs.push.apply(boundArgs, args);
  return new(Parent.bind.apply(Parent, boundArgs));
}

function test(name) {
  return Function('reConstruct', 'return ' + (function Child() {

    // the super call bit
    var
    instance = reConstruct.call(
      this,
      Object.getPrototypeOf(Child),
      arguments,
      Child
    ),
    type = instance ? typeof instance : '',
    self = (type === 'object' || type === 'function') ?
      Object.setPrototypeOf(instance, Child.prototype) :
      this
    ;

    // the rest of the constructor body
    console.log(self.constructor.name);

    // at the end or instead of empty returns
    return self;
  }).toString().replace(/Child/g, name))(reConstruct);
}

var A = function A() {
    console.log(this.constructor.name);
};

var B = ec5end(test('B'), A);
var C = ec5end(test('C'), B);

function ec5end(C, P) {
  C.prototype = Object.setPrototypeOf(
    C.prototype,
    P.prototype
  );
  return Object.setPrototypeOf(C, P);
}

so basically this is the change:

var
  reConstruct = typeof Reflect === 'object' ?
    Reflect.construct :
    function (Parent, args, Child) {
        var boundArgs = [this];
        boundArgs.push.apply(boundArgs, args);
        return new(Parent.bind.apply(Parent, boundArgs));
    }
;

Outstanding problems: the log is A, B, C but it's a relatively minor issue compared with how much is currently "broken" the resulting transpiled code.

[WebReflection]

BTW do you know about https://github.com/loganfsmyth/babel-plugin-transform-builtin-extend?

Yes, IIRC it doesn't scale well with HTMLElement, XMLHttpRequest or some other native + I am not sure I should put every single known class in the mix and maintain that list each time I extend something.

Is this how it works?

[loganfsmyth]

@WebReflection We could certainly expand the functionality of that module, either with a whitelist, or the ability to just say "all globals" or something, thoughts?

There's the possibility of a solution like #3582, but it's unlikely that we could land a generic solution like you have proposed because calling Object.setPrototypeOf would dramatically decrease instance creation performance for all non-native classes unless we can get a sufficiently good heuristic to decide when to apply it.

[WebReflection]

I will find out a way to generate at runtime the right code accordingly with the kind of extend is needed. The entry point to solve this is a native parent, subclasses after that can already be somehow simplified. The setPrototypeOf if a parent has an early return is mandatory anyway so I am not sure on real-world code how much of an impact would be.

I'd love to have data though.

[WebReflection]

Update
Apologies I've been quite busy these days but there's an apparently common and cross platform standard that makes it trivial to understand if setPrototypeOf should be invoked, in order to upgrade an instance, or not.

function requiresUpgrade(Class) {
    return  !Object.getOwnPropertyDescriptor(
                Class,
                'prototype'
            ).writable;
}

This seems to be consistent with all natives but also with native ES6+ classes.
However, native ES6+ classes are basically a non issue in Babel because these are transpiled into regular functions.

This makes it possible to decide at definition time if the transpiled function needs more operations or it can simply use the current logic.

Thoughts? If this looks OK I can move forward with the rest of the implementation details.

[smalluban]

I am not sure if this is directly connected to this issue, but Babel class transpilation breaks using native customElements API. HTMLElement constructor has to be called with new operator, so in transpiled code Reflect has to be used.

I have found an article The JS super (and transpilers) problem with possible solution (last code example).

I created two codepen examples.

Babel transpiled class (http://codepen.io/smalluban/pen/wzgoON?editors=0012):

• Chrome 54: "Failed to construct 'HTMLElement': Please use the 'new' operator, this DOM object constructor cannot be called as a function."
• Firefox 48: "created element"
• IE 11: "created element"

Firefox/IE uses customElements polyfill, so it works well - there is no problem with target.new, but Chrome requires that.

Custom transpiled class (http://codepen.io/smalluban/pen/JREbaW?editors=0012):

• Chrome 54: "created element"
• Firefox 48: "created element"
• IE 11: "created element"

In this example if Reflect API is present, it's used to call parent constructor, and then creating custom element works in both native and polyfilled versions.

I am not sure how this will work with "normal" class definition where special behavior is not required.

[WebReflection]

@smalluban yes, it's related, and Custom Elements extends are indeed the reason I came here.

[NekR]

Without Reflect.construct:
class Class extends Date {};
+new Class(); // => ?

Even if some inheritance is done wrong, there always is this:

function Test() {}
Test.prototype.valueOf = function() { return 3; };

+new Test(); // 3

// and even
Date.prototype.valueOf = function() { return 5; };

+new Date(); // 5

[WebReflection]

It looks like we all agree on this point: WICG/webcomponents#587 (comment)

Not even browser vendors developers can workaround the current broken state when it comes to super.

I won't have time soon to solve this issue, I'd like to understand if it has any priority though so at least I can better reply to people asking me why my polyfill is broken ( when the issue is them using Babel :-( )

Thanks for any sort of ETA / outcome

[lukasoppermann]

@zloirock So is there a fix coming for this? It basically makes web components / custom elements unusable. 😢

[WebReflection]

Once again, the idea behind (but I'm not familiar with Babel and I don't have time now to push a PR) is to change the transformation so that writing this class:

class List extends Array {
  constructor(a, b, c) {
    super(a, b);
    this.push(c);
  }
}

should produce the following:

"use strict";

function _classCallCheck(instance, Constructor) { if (!(instance instanceof Constructor)) { throw new TypeError("Cannot call a class as a function"); } }

function _possibleConstructorReturn(self, call) { if (!self) { throw new ReferenceError("this hasn't been initialised - super() hasn't been called"); } return call && (typeof call === "object" || typeof call === "function") ? call : self; }

function _inherits(subClass, superClass) { if (typeof superClass !== "function" && superClass !== null) { throw new TypeError("Super expression must either be null or a function, not " + typeof superClass); } subClass.prototype = Object.create(superClass && superClass.prototype, { constructor: { value: subClass, enumerable: false, writable: true, configurable: true } }); if (superClass) Object.setPrototypeOf ? Object.setPrototypeOf(subClass, superClass) : subClass.__proto__ = superClass; }

var List = function (_Array) {
  _inherits(List, _Array);

  var _retrieveThis = Object.getOwnPropertyDescriptor(_Array, 'prototype').writable ?
    // user defined class or function
    function (Parent, a, b) {
        return _possibleConstructorReturn(this, Parent.call(this, a, b));
    } :
    // native class
    function (Parent, a, b) {
        // eventually it needs a Reflect and Object.setPrototypeOf polyfill upfront
        return Object.setPrototypeOf(Reflect.construct(Parent, [a, b], List), List.prototype);
    };

  function List(a, b, c) {
    _classCallCheck(this, List);

    var _this = _retrieveThis.call(this, (List.__proto__ || Object.getPrototypeOf(List)), a, b);

    _this.push(c);
    return _this;
  }

  return List;
}(Array);

At this point the following code would work as expected:

var l = new List(1, 2, 3);
l.length; // 3
l; // [1, 2, 3]
l instanceof Array; // true
l instanceof List; // true

The check Object.getOwnPropertyDescriptor(Class, 'prototype').writable returns true with everything userland, transpiled, function, and false with native classes (since in Babel only native classes are preserved, all others are transpiled).

Above transformation would work with every modern browser that supports native classes and Reflect, which would be already a very wide variety of devices and browsers.

In alternative, we might need a solution that fallbacks in order browsers when it comes to simulate Object.setPrototypeOf and / or Reflect.construct, trying to preserve the logic used in more articulated cases.

Although having the direct super working would be already a solution for I believe 90% of use cases since at the end of the day in Babel we cannot define native classes so that it's only possible to have a single Reflect.construct call per inheritance chain.

Does this help anyone speeding up the release of a patch for this issue?

[stephanbureau]

Hi,

As a workaround on my side, I use github:WebReflection/document-register-element as polyfill to import. Then, the only issue I still encountered is attributeChangedCallback which is not called when changing attributes values, not perfect I know.
connectedCallback or disconnectedCallback methods are correctly called though.

For info, I am using jspm 0.17.0-beta.31 with babel 6.16.0.

Here's an example with a basic class:

in ES6

class Test extends HTMLElement {
    static get TAG() {
        return 'my-test';
    }
    connectedCallback() {
        this.innerHTML = 'content';
    }
}

Transpilled with babel runtime

Test = function (_HTMLElement) {
    _inherits(Test, _HTMLElement);

    function Test () {
        _classCallCheck(this, Test);

        return _possibleConstructorReturn(this, Object.getPrototypeOf(Test).apply(this, arguments));
    }

    _createClass(Test, [{
        key: 'connectedCallback',
        value: function connectedCallback() {
            this.innerHTML = 'content';
        }
    }], [{
        key: 'TAG',
        get: function get() {
            return 'my-test';
        }
    }]);

    return Test;
}(HTMLElement);

Hope it helps a bit.

[WebReflection]

@stephanbureau remember in V1 there are changes so that you need to specify upfront attributes to listen to.

class MyDom extends HTMLElement {
  static get observedAttributes() {
    return ['country'];
  }
  attributeChangedCallback(name, oldValue, newValue) {
    // react to changes for name
    alert(name + ':' + newValue);
  }
}

var md = new MyDom();
md.setAttribute('test', 'nope');
md.setAttribute('country', 'UK'); // country: UK

[stephanbureau]

Amazing, that was the missing piece for me. Thanks. Sorry for the noise on the issue.

[WebReflection]

@aaronshaf how does that help with the constructor problem ?

[zloirock]

By the way, I'm strongly in favour of adding something like that - #1172, but not so complex version (a special case for built-ins without multilevel subclassing). It was added in early versions on babel@5 and I don't know why it was removed.

[WebReflection]

@zloirock have you seen my proposed changes ?
#4480 (comment)

You can compare them directly with the current output generated by

class List extends Array {
  constructor(a, b, c) {
    super(a, b);
    this.push(c);
  }
}

Current

"use strict";

function _classCallCheck(instance, Constructor) { if (!(instance instanceof Constructor)) { throw new TypeError("Cannot call a class as a function"); } }

function _possibleConstructorReturn(self, call) { if (!self) { throw new ReferenceError("this hasn't been initialised - super() hasn't been called"); } return call && (typeof call === "object" || typeof call === "function") ? call : self; }

function _inherits(subClass, superClass) { if (typeof superClass !== "function" && superClass !== null) { throw new TypeError("Super expression must either be null or a function, not " + typeof superClass); } subClass.prototype = Object.create(superClass && superClass.prototype, { constructor: { value: subClass, enumerable: false, writable: true, configurable: true } }); if (superClass) Object.setPrototypeOf ? Object.setPrototypeOf(subClass, superClass) : subClass.__proto__ = superClass; }

var List = function (_Array) {
  _inherits(List, _Array);

  function List(a, b, c) {
    _classCallCheck(this, List);

    var _this = _possibleConstructorReturn(this, (List.__proto__ || Object.getPrototypeOf(List)).call(this, a, b));

    _this.push(c);
    return _this;
  }

  return List;
}(Array);

Improved

"use strict";

function _classCallCheck(instance, Constructor) { if (!(instance instanceof Constructor)) { throw new TypeError("Cannot call a class as a function"); } }

function _possibleConstructorReturn(self, call) { if (!self) { throw new ReferenceError("this hasn't been initialised - super() hasn't been called"); } return call && (typeof call === "object" || typeof call === "function") ? call : self; }

function _inherits(subClass, superClass) { if (typeof superClass !== "function" && superClass !== null) { throw new TypeError("Super expression must either be null or a function, not " + typeof superClass); } subClass.prototype = Object.create(superClass && superClass.prototype, { constructor: { value: subClass, enumerable: false, writable: true, configurable: true } }); if (superClass) Object.setPrototypeOf ? Object.setPrototypeOf(subClass, superClass) : subClass.__proto__ = superClass; }

var List = function (_Array) {
  _inherits(List, _Array);

  var _retrieveThis = Object.getOwnPropertyDescriptor(_Array, 'prototype').writable ?
    // user defined class or function
    function (Parent, a, b) {
        return _possibleConstructorReturn(this, Parent.call(this, a, b));
    } :
    // native class
    function (Parent, a, b) {
        // eventually it needs a Reflect and Object.setPrototypeOf polyfill upfront
        return Object.setPrototypeOf(Reflect.construct(Parent, [a, b], List), List.prototype);
    };

  function List(a, b, c) {
    _classCallCheck(this, List);

    var _this = _retrieveThis.call(this, (List.__proto__ || Object.getPrototypeOf(List)), a, b);

    _this.push(c);
    return _this;
  }

  return List;
}(Array);

The only overhead for non native constructors is during class definition time (so once per Application lifecycle) through:

Object.getOwnPropertyDescriptor(_Array, 'prototype').writable

Hopefully, that's practically irrelevant for real-world projects.

console.time('gOPD');
for(let i = 0; i < 1000; i++)
  Object.getOwnPropertyDescriptor(Array, 'prototype').writable;
console.timeEnd('gOPD');
// gOPD: 0.582ms for thousand classes

[zloirock]

@WebReflection looks good, but I'm not sure about Object.getOwnPropertyDescriptor(Array, 'prototype').writable in different browsers, need to check it.

[WebReflection]

@zloirock it's in specs, it should be consistent with every ES5 compatible browser and, if polyfilled properly in ES5-shim, down to older browsers/engine too.

Since babel transform every class into ES3 compatible functions, that check will always be true on user defined classes transpiled through Babel, and false for native constructors (unless these are wrapped, in these cases the wrap should set the prototype as non writable)

[zloirock]

@WebReflection I don't see any fixes for this difference in es5-shim.

[WebReflection]

@zloirock that's eventually a possible bug/fix for ES5 shim. Meanwhile, all ES5 compatible browsers would benefit from this update ... right?

[WebReflection]

FYI the plugin has been updated after a fix related to N level inheritance. There are also tests now to validate it works on NodeJS.
https://github.com/WebReflection/babel-plugin-transform-builtin-classes

Custom Elements also seem to work without any issue with or without my polyfill.

I believe all my tests are using the Reflect.construct available in all targets I am checking, but if you could confirm it's OK even on older browsers/engines that'd be awesome.

Best Regards

[hzoo]

@WebReflection not sure what the others think yet, but we could move this into the main transform and make it an option there? Seems like it could be a relatively simple port

[WebReflection]

@hzoo dunno what to answer. I have no idea how Babel proceeds but I'm sure the world would appreciate a fix in core for the most annoying untranspiled thing ever 😄

I've tested here and there my latest changes and while I'm sure there will be some edge weird case to eventually document, it worked well for all my Custom Elements and other native extends cases.

I've also tested browsers without Reflect support and it looks like everything is just fine.

To me it'd be an OK 👍 to proceed, but it also needs some documentation around, 'cause developers need to understand they have to specify upfront which native constructor they'd like to extend.

Please let me know what I can do to move forward, thanks.

[WebReflection]

Maybe we should close this as won't fix since there's no activity whatsoever around this bug?

I sometimes go through my GitHub issues and this one looks like one of those that will be there forever.

Please let me know if there's anything I should do or if I can just drop it, thanks.

[nicolo-ribaudo]

We should at least add a link to the babel-plugin-transform-classes which suggests the users your plugin.
Anyway, if you are ok with it, I can work on merging the two plugins.

[WebReflection]

Anyway, if you are ok with it, I can work on merging the two plugins.

I am OK with it, but not sure which two plugins you are talking about.

Everything but keeping this pointlessly open works for me.

[FND]

Thanks everyone for making this happen - and of course @WebReflection in particular for figuring this out in the first place and staying on the ball.

For users who aren't familiar with Babel's internals, AFAICT the fix in #7020 will be included by default with the upcoming Babel v7.0? (If you're using something like @babel/preset-es2015 or @babel/preset-env, that is - at least those presets seem to depend on @babel/plugin-transform-classes.)