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

ES5 consideration for custom elements #423

Closed
rniwa opened this issue Mar 7, 2016 · 20 comments
Closed

ES5 consideration for custom elements #423

rniwa opened this issue Mar 7, 2016 · 20 comments

Comments

@rniwa
Copy link
Collaborator

rniwa commented Mar 7, 2016

We think this is okay but I'll note that the new constructor based custom elements API is incompatible with ES5 because there is no mechanism to set new.target in ES5 without use of Reflect.construct.

This is probably non-issue though because by the time custom elements API will be implemented by all browsers, Reflect.construct would certainly be available everywhere.

This is just FYI.

@annevk
Copy link
Collaborator

annevk commented Mar 11, 2016

@domenic is it worth adding a note somewhere? I don't really care either way.

@domenic
Copy link
Collaborator

domenic commented Mar 11, 2016

I don't really think so. As @rniwa notes, every browser that implements custom elements also implements class syntax.

We can close. Using issue tracker for PSAs is a good idea though.

@domenic domenic closed this as completed Mar 11, 2016
@rajsite
Copy link

rajsite commented Mar 22, 2016

Does this mean that custom elements v1 will not be compatible with ES5 style "classes"?

For example the custom tag example could NOT be written as the following with custom elements v1:

var FlagIcon = function () {
  HTMLElement.call(this)
  this._countryCode = null
}

FlagIcon.prototype = Object.create(HTMLElement.prototype)
FlagIcon.prototype.contructor = FlagIcon

FlagIcon.prototype.observedAttributes = function () {
  return ["country"]
}

FlagIcon.prototype.attributeChangedCallback(name, oldValue, newValue) {
  // name will always be "country" due to observedAttributes
  this._countryCode = newValue
  this._updateRendering()
}

FlagIcon.prototype.connectedCallback() {
  this._updateRendering()
}

Object.defineProperty(FlagIcon.prototype, "country", {
  get: function () {
    return this._countryCode
  },
  set: function (v) {
    this.setAttribute("country", v)
  }
});

FlagIcon.prototype._updateRendering() {
    // Left as an exercise for the reader. But, you'll probably want to
    // check this.ownerDocument.defaultView to see if we've been
    // inserted into a document with a browsing context, and avoid
    // doing any work if not.
  }
}

If it is the case that custom elements v1 will not support ES5 style classes I am concerned that it will make it unnecessarily difficult for many applications that want to begin the process of migrating to custom elements.

Assuming a developer wants to incrementally support modern browser features (using tools like transpilers and polyfills), the following scenarios come to mind:

  1. Browser is a fairly modern HTML5, ES5 browser:
    requires a custom element v1 polyfill (the polyfill would support ES5 style "classes")
    requires a ES6 class transpiler that will create ES5 style classes
  2. Browser is a modern HTML5, ES5 and partial ES6 browser:
    requires a custom element v1 polyfill (polyfill would support ES6 classes and ES5 style "classes")
    does not require ES6 class transpiler
  3. Browser is a bleeding edge HTML5, ES6 browser:
    does not require a custom element v1 polyfill
    does not require ES6 class transpiler

As a developer who wants to support custom elements in a way that is forward looking and yet compatible with modern browsers, I can choose to write in ES6 classes against the custom elements v1 api. For deployment, I transpile ES6 classes to ES5 style "classes" and include a custom element v1 polyfill.

In the ideal situation, browsers that support custom element v1 natively will not utilize the custom element v1 polyfill and consume the ES5 style "classes" with the native custom element v1 api.

However, if ES6 style classes are required, then deployment becomes much more complex. One option would be to use feature detection to selectively serve two versions of all elements (one version with ES6 classes to the bleeding edge browsers and one version with ES5 style "classes" to the broad-base). Or even worse, create a single build that utilizes the polyfill in browsers that do not support custom element v1 and force the polyfill on bleeding edge browsers as well. The later scenario seems to be the more likely outcome because of the significantly reduced difficulty of distributing a single build while simultaneously adopting a larger user base.

If the idea is that requiring ES6 classes is an acceptable migration burden for early adopters utilizing custom elements v0 and for new users of the platform, I believe this decision will unnecessarily punish new adopters and encourage non-native and non-spec compliant polyfill implementations. Considering that custom elements behavior is so foundational to the whole idea of api interoperability between web components and frameworks consuming web components, encouraging adoption of spec compliant implementations should be a high priority.

Alternatively, I am completely off the rails and have misinterpreted both the current drafts of the custom element v1 specification and this Github issue. If that is the case and ES5 style "classes" are to be supported then some of the phrasing such as:

A parameter-less call to super() must be the first statement in the constructor body...

is a bit unclear and I can file a separate issue for those cases. If you made it this far, thank you for taking the time to read this post and I look forward to your thoughts.

@rniwa
Copy link
Collaborator Author

rniwa commented Mar 22, 2016

HTMLElement.call(this) would not work but Reflect.construct(HTMLElement, [], FlagIcon) would.

All browsers that support custom elements v1 API would support Reflect.construct. The contrapositive is that all browsers that don't support Reflect.construct would not support custom elements v1.

So, you can write a wrapper around it which calls Reflect.construct to use native support for custom elements on "modern" browsers, and use custom elements polyfill (e.g. setting __proto__) on others.

@rajsite
Copy link

rajsite commented Mar 22, 2016

Thank you for the quick reply!

To take it a step further, the pattern of using Reflect.construct would be required for all ES5 style derived "classes" that want to extend base constructors that create exotic objects. This is probably not unique to HTMLElement, but also to Array, etc. Does that seem fair?

If that is the case, the spec makes the following note:

For now, the HTMLElement constructor cannot be invoked directly. It only works when used via a super() call inside a custom element constructor.

Is this note implying that:

  • Reflect.construct will not be required in the future as a non-exotic object will be returned from the HTMLElement constructor or
  • Is this implying that browsers do not currently have Reflect.construct, but in the future browsers could invoke the base constructor through either a call to super() or using Reflect.construct?

Side note: I found the Exploring JS: Classes chapter very helpful for this discussion

@domenic
Copy link
Collaborator

domenic commented Mar 22, 2016

I don't believe that note is implying either of those things. What about it do you find unclear?

@rniwa
Copy link
Collaborator Author

rniwa commented Mar 22, 2016

The spec doesn't imply either. super() is basically a syntactic sugar of Reflect.construct.

Reflect.construct will not be required in the future as a non-exotic object will be returned from the HTMLElement constructor

This is certainly not the case. HTMLElement constructor will continue to return an exotic object for the foreseeable future.

Is this implying that browsers do not currently have Reflect.construct, but in the future browsers could invoke the base constructor through either a call to super() or using Reflect.construct?

All browsers that do support custom elements will support Reflect.construct. To put it differently, there is no known implementation of custom elements v1 but all major browsers (Edge, Firefox Nightly, Chrome Canary, and WebKit) support Reflect.construct already so it's inconceivable that we'd end up in a situation whereby which we have custom elements support but not Reflect.construct.

In other browsers that don't support custom elements or Reflect.construct, library and frameworks authors can polyfill both features to make their custom elements polyfill work.

Perhaps the spec could be updated to also mention Reflect.construct for clarity.

@rajsite
Copy link

rajsite commented Mar 22, 2016

Maybe it is more of a phrasing issue. By saying:

For now, the HtmlElement contructor...

It seems to imply that something will be changing in the future, but it is not very clear what would change.

@domenic
Copy link
Collaborator

domenic commented Mar 22, 2016

What can change is that we might allow something like new HTMLElement("section"). See whatwg/html#896.

@rajsite
Copy link

rajsite commented Mar 22, 2016

@domenic Mentioning that reason would be clearer about what is being implied

For now, the HTMLElement constructor cannot be invoked directly as the corresponding tag name that would be used is not well specified.

@rniwa I think it would be appreciated if a reference to calling the HTMLElement constructor via either super() or Reflect.construct was included.

The HTMLElement constructor can only be called from inside a custom element constructor, for example via invoking super() or using Reflect.construct

Either that or just loosening the language a bit

The HTMLElement constructor can only be called from inside a custom element constructor, for example via an invocation of super()

@rniwa
Copy link
Collaborator Author

rniwa commented Mar 23, 2016

That's probably a good change. @domenic : could you make that spec change for us?

@domenic
Copy link
Collaborator

domenic commented Mar 23, 2016

I think it's less accurate than what we have now. If you're looking for precision it should be something along the lines of "the HTMLElemwnt constructor cannot be invoked unless NewTarget is set to a custom element constructor." Saying that it can be invoked "inside", " for example by super()", is just incorrect.

I'd like further clarity on exactly what is wrong with the current phrasing.

@rniwa
Copy link
Collaborator Author

rniwa commented Mar 23, 2016

I think the problem is that the current spec specifically says we must use super(), which is not true. I'd be fine with saying that new.target must be set although it's probably less clear than saying author has to use either super() or Reflect.construct.

@trusktr
Copy link

trusktr commented Apr 11, 2016

I didn't read the whole thread, but currently, Custom Elements are compatible with ES5 constructor-pattern classes like this:

function MyElement() {
    // The only caveat: just don't do anything in this constructor.
}

MyElement.prototype = Object.create(HTMLElement.prototype)
MyElement.prototype.createdCallback = function createdCallback() {
    // Instead, do your "constructor" logic here.
}

MyElement = document.registerElement('my-element', )
// ^ This assignment is required if you'd like to use `new MyElement` directly
// instead of document.createElement('my-element')

export default MyElement

This also works with ES6 classes:

class MyElement extends HTMLElement {
    createdCallback() {
        // Do your "constructor" logic here.
    }
}

MyElement = document.registerElement('my-element', )
// ^ This assignment is required if you'd like to use `new MyElement` directly
// instead of document.createElement('my-element')

export default MyElement

then in another file

import MyElement from './my-element'

let el = document.createElement('my-element')
// or
let el = new MyElement

It'd be cool if it just worked properly with (at least) ES6 classes:

class MyElement extends HTMLElement {
    constructor() {
        // Do your actual constructor logic here.
    }
}

document.registerElement('my-element', MyElement)
// ^ With no assignment needed.

export default MyElement

then just

import MyElement from './my-element'

let el = new MyElement

// document.createElement() not needed, but can exist for backwards compatibility.

I don't see why this can't work with any constructor (ES5 constructor-pattern classes for example). Anything's possible!

@justinfagnani
Copy link
Contributor

@trusktr this is exactly what's happening. This thread is only about purely ES5 style constructors which call their super constructor with Function.call. In this case new.target won't be set.

I think this is mainly a concern for output of compilers and code that aims to be compatible with both native implementations and polyfills on browsers without new.target. I wouldn't worry too much though, it's possible to make a small shim that overrides HTMLElement and sets new.target to this.constructor, which is usually set to the right function in ES5 inheritance patterns.

@rniwa
Copy link
Collaborator Author

rniwa commented Apr 11, 2016

FYI, new.target is readonly (i.e. not assignable). You need to use Reflect.construct to set new.target instead.

@justinfagnani
Copy link
Contributor

@rniwa yes, that's what the shim I have does.

@rajsite
Copy link

rajsite commented Dec 22, 2016

For anyone stumbling across this, newer discussion on this topic can be found here: #587

In particular: #587 (comment)

@trusktr
Copy link

trusktr commented Sep 30, 2017

Everyone, the strict enforcement of using new means that transpilers can not compile to ES5 without using Reflect.construct because Reflect.construct won't work in anything below Edge 12.

This is a bad because not everyone is off of IE 10 or 11 yet, and there's no way to transpile this to a format that doesn't use new.target or Reflect.construct.

What this means is that people transpiling this to ES5 and using a Custom Elements v1 and Reflect polyfill will get lucky that it works in IE10, but then they'll get this error in new browsers like Chrome:

Uncaught TypeError: Failed to construct 'HTMLElement': Please use the 'new' operator, this DOM object constructor cannot be called as a function

@trusktr
Copy link

trusktr commented Sep 30, 2017

For example, see how bad this is with Buble output. Run the output code in Chrome.

This is not good! It makes things very difficult, especially in a time when many people still have to support non-native-ES2015+ environments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants