WebReflection / document-register-element

A stand-alone working lightweight version of the W3C Custom Elements specification
ISC License
1.13k stars 116 forks source link

How do I use this in Edge? #121

Closed trusktr closed 7 years ago

trusktr commented 7 years ago

I'm trying to get Custom Elements working in Edge 14, and if I use vanilla ES class syntax, or ES5 transpiled classes, I have no luck. Either way, I get the error SCRIPT5002: Function expected which I'm sure you may have seen before. The stack trace leads to this part of document-register-element:

    safeProperty(proto, CREATED_CALLBACK, {
        value: function () {
          if (justCreated) { justCreated = false; }
          else if (!this[DRECEV1]) {
            this[DRECEV1] = true;
            new Class(this); // <-------- stack trace points here --------------
            if (CProto[CREATED_CALLBACK])
              { CProto[CREATED_CALLBACK].call(this); }
            var info = constructors[nodeNames.get(Class)];
            if (!usableCustomElements || info.create.length > 1) {
              notifyAttributes(this);
            }
          }
      }
    });

The trace starts right there at the new Class(this); call. Every other function in the trace is a constructor from my Custom Element's prototype chain, and eventually (through super() calls) it gets to the constructor that calls HTMLElement as super() or HTMLElement.call(this) depending on if transpiled or not. Either way it gives me that same SCRIPT5002 error.

After getting this error, I can verify that CustomElementRegistry does not exist on window yet. It seems like this error causes the polyfill to fail before it ever assigns anything onto window. Why is this? Why does my class get instantiated by the polyfill before it ever adds anything to window?

I'm importing it as import "document-register-element".

I'm using version 1.7.0.

In Chrome, everything works with native class, but fails with the transpiled version (it would need Justin's native-shim adapter).

What do you recommend to get this working in Edge 14?

trusktr commented 7 years ago

Here's a reproduction (open in Edge): https://codepen.io/anon/pen/LzePxV

(You can use http://crossbrowsertesting.com to open code pens in Edge remotely for free)

trusktr commented 7 years ago

In that reproduction, the single bundle loaded by script tag includes the polyfill. This bundle is built from the entry point of infamous, src/index.js The file src/html/index.js imports the polyfill.

I would guess that the polyfill is executed during module evaluation (i.e. when the bundle is evaluated). I would also think that my classes would not be instantiated during that time, unless maybe the polyfill is trying to upgrade elements that already exist in DOM, and since this fails, the polyfill fails and nothing is assigned to window?

WebReflection commented 7 years ago

If you use transpilers you’re responsible for doing it right.

For Babel, use this: https://github.com/WebReflection/babel-plugin-transform-builtin-classes

trusktr commented 7 years ago

I'm using Buble, looks like Buble does it wrong then, it's "batteries included".

So just to make sure I understand, this polyfill requires transpilation, not just polyfill by itself?

WebReflection commented 7 years ago

P.S. this polyfill does feature detections but as CJS module can be initialized in various ways as described in the README.

This module is used by Google AMP, StencilJS and Aframe without any issue.

Nothing from webcomponents.js will land here. If you like webcomponents and that works for you, please use that instead.

WebReflection commented 7 years ago

I don’t know what’s Buble and I don’t believe in “batteries included” marketing as a proof of infallible software.

trusktr commented 7 years ago

I was hoping to use native classes (rather than ES5 classes, transpiled or not). So to use document-register-element in Edge, I must transpile classes to ES5 using your babel plugin first?

trusktr commented 7 years ago

I wonder how much overhead calling Reflect.construct adds, during instantiation of every instance of every class.

WebReflection commented 7 years ago

to use document-register-element in Edge, I must transpile classes to ES5 using your babel plugin first?

no, you just need to read the readme.

trusktr commented 7 years ago

I missed that part. I just gave this a shot, but no luck. I basically have the following, with no transpilation, leaving class syntax as is:

export default
function WebComponent(base) {
  base = base || HTMLElement
  return class WebComponent extends base {
    constructor(self) {
      const _this = super(self) // <-- HERE, your suggestion
      // use _this
      return _this
    }
  }
}

then in another file

import WebComponent from './WebComponent'
export default
class CoolElement extends WebComponent(window.HTMLElement) { ... }
import 'document-register-element'
customElements.define('cool-element', CoolElement)

Finally when I use the <cool-element> in the DOM, I get the familiar error in Chrome:

Failed to construct 'HTMLElement': Please use the 'new' operator, this DOM object constructor cannot be called as a function.
trusktr commented 7 years ago

I changed

import 'document-register-element'
customElements.define('cool-element', CoolElement)

to

import installCE from 'document-register-element/pony'
installCE(window, 'force')
customElements.define('cool-element', CoolElement)

but no luck that way either.

WebReflection commented 7 years ago

if you get Please use the 'new' operator most likely you are transpiling code or the polyfill wasn't available before you extended HTMLElement or your WebComponent mixin, which I've no idea what is it, does something wrong, like not using the same approach mentioned as caveat.

trusktr commented 7 years ago

Ah, finally got it working (with a mix of changing all the constructors to account for the constructor(self) argument and making sure the polyfill was loaded first (without babel-plugin-transform-builtin-classes).

I was under the impression that using babel-plugin-transform-builtin-classes would prevent me from having to accept a self arg in all of the constructors. Is this the case?

trusktr commented 7 years ago

Confirmed, constructor(self) is not required when using babel-plugin-transform-builtin-classes and native CE v1.

But, if I want to use your CE v1 polyfill, is there no way to use it without self in constructor(self)? I'd like not to have to define a self parameter in all constructors.

trusktr commented 7 years ago

I don’t know what’s Buble

It's quite awesome. The output is like hand-written. Unfortunately, it might not conform 100% to ES6+ spec (on purpose).


I ended up getting webcomponents + native-shim working in Edge 12+, so sticking to that for now. Thanks for all the advice. I got this working too, but the requirement for self contructor param is stopping me.

WebReflection commented 7 years ago

But, if I want to use your CE v1 polyfill, is there no way to use it without self in constructor(self) ?

Invoking a super with arguments is nothing invented here.

class B extends A {
  constructor(...args) {
    super(...args);
    this.whatever = Math.random();
  }
}

Above class will work as meant but if you don't like the ...args "boilerplate" then you can follow the README: https://github.com/WebReflection/document-register-element#skipping-the-caveat-through-extends

// base class to extend, same trick as before
class HTMLCustomElement extends HTMLElement {
  constructor(_) { return (_ = super(_)).init(), _; }
  init() { /* override as you like */ }
}

// create any other class inheriting HTMLCustomElement
class MyElement extends HTMLCustomElement {
  init() {
    // just use `this` as regular context reference
    this.addEventListener('click', console.log);
    // no need to return it
  }
}
trusktr commented 7 years ago

Either way, I either have to modify all my constructor definitions (and tells end users to always follow the pattern) or rename all my constructors to something else (and tell users of the library not to use constructor).

Either case would probably be fine in reality, if a library is awesome, people are willing to follow a convention in order to use it.

With webcomponents/custom-elements, this constructor mod isn't required, but unfortunately there's the ugly situation of "how do I include or not include native-shim conditionally without causing syntax errors or it failing due to CSP headers"?

trusktr commented 7 years ago

What happens when the element user wants to instantiate the class directly (I don't see it in the README)?

import SomeElement from './some-place'
customElement.define('some-element', SomeElement)

new SomeElement( /* ? */ )

What needs to be passed in?

WebReflection commented 7 years ago

Nothing. The upgrade happens only on nodes live on the document .

trusktr commented 7 years ago

So we would need to do?

const elm = new SomeElement(undefined, other, args)
document.body.appendChild(elm)

?

WebReflection commented 7 years ago

LOL .. you don't ever pass anything to Custom Elements constructors, because if these are already on the DOM nothing will be passed.

document.body.innerHTML = '<my-el></my-el>';

customElements.define('my-el', class extends HTMLElement {
  constructor(tro, lo, lol, ol) {
    console.log('even in Chrome nothing will ever pass through');
  }
});

It's a very bad choice to create Custom Elements dependent on any constructor signature, all you have to do is to offer an init, a setup, or a readyCallback() in your class, like V0 was offering a createdCallback because constructor was unreachable (I wish that was still the case).

WebReflection commented 7 years ago

TL;DR it's easier for everyone if you completely forget about the constructor when it comes to custom elements. The constructors has many quirks developers are not aware of. The super call is mandatory, not true for other callbacks, the element might not even be ready/live/upgraded, it needs to wait for attributeChangedCallback or connectedCallback to operate.

As example, you try to innerHTML = 'anything' on a non upgraded element in the constructor, it fails brutally.

Check CustomTag API: https://github.com/WebReflection/custom-tag#api

Check HyperHTMLElement: https://github.com/WebReflection/hyperHTML-Element#the-class

stop thinking Custom Elements are like any other class in JS because they are not. The upgrade mechanism is a pain in the specs for various reasons: avoid as much as you can!

trusktr commented 7 years ago

because if these are already on the DOM nothing will be passed.

That's true!

But interestingly, I'm designing classes that are designed to be used imperatively (no DOM, outside the browser, for example in Node with a direct OpenGL renderer with no HTML engine) or as Custom Elements (the classes are class-factory-mixed on top of auto-generated HTMLElement-extending classes).

The classes, when used imperatively, can take initial values, f.e.:

new SomeThing({
  someOption: 'foo',
  otherOption: 'bar',
})

These classes define a scene that can be rendered with WebGL or OpenGL (f.e. in AltspaceVR with no DOM).

They can also be used in a DOM:

document.body.innerHTML = `
  <some-element
    someOption="foo"
    otherOption="bar"
  >
    ...
  </some-element>
`

SomeThing.html.define('some-element') // uses customElements.define internally

In either case, used in DOM or with no DOM at all, instance options can be modified imperatively:

const something = new SomeThing()

something.someOption = "foo"
something.otherOption = "bar"

It could be possible that I can just drop the constructor args, but currently this has a downside that initialization of values will be less efficient.


In case you're curious, AltspaceVR uses Three.js with a custom renderer that renders to OpenGL directly (not WebGL, there's no browser involved), and these components that I'm making work with it because they are built on Three.js, but they can also render to WebGL (on Three.js), and they can also be controlled via auto-generated custom elements so that you can use React, Angular, etc, to define scenes.

trusktr commented 7 years ago

In other words, I'm not thinking only about DOM. DOM is only an optional interface for controlling the components I'm making.

trusktr commented 7 years ago

Maybe, in my base (auto-generated) class, I can do something like:

constructor(arg) {
  let options = null;
  if (/* arg is an Element */) {
    // the instance was instantiated by the HTML engine as a DOM element
    options = {}
    this._instantiatedFromDOM = true // perhaps useful in extending classes
  }
  else { // arg is manually passed constructor options
    options = arg
    this._instantiatedFromDOM = false // perhaps useful in extending classes
  }
}
// then it can work both ways

document.body.appendChild(new SomeThing({ foo: 'foo', bar: 'bar' }))
// or
document.body.innerHTML = "<some-thing foo="foo" bar="bar"></some-thing>"
trusktr commented 7 years ago

I suppose I need to use the "force" option for installCE, otherwise this will fail in native CE v1. Some day when I switch to native v1, then I'd need to refactor this just a little bit.

WebReflection commented 7 years ago

The only thing you need to do with the constructor is to invoke super with the node, if any.

class AltspaceVR extends HTMLElement {
  constructor(self) {
    const args = [].concat(self && 'nodeType' in self ? self : []);
    super(...args);
  }
}

customElements.define('altspace-vr', AltspaceVR);

const avr = new AltspaceVR;

If you expose this class, everyone using your library can extend that instead and forget about the super or its arguments.

In few words, I don't understand what is your issue. You can provide any class that does whatever you want upfront, right?

trusktr commented 7 years ago

I was just mentioning that I need to handle it somehow, but I don't have to if I use native CE v1 because passing anything to HTMLElement in v1 has no effect.

WebReflection commented 7 years ago

you were also saying this:

I suppose I need to use the "force" option for installCE, otherwise this will fail in native CE v1.

which is not the case

trusktr commented 7 years ago

Oh, I see. Your way just doesn't pass anything in unless it's an element (which doesn't do anything in native v1 anyways). 👍 Thanks!

trusktr commented 7 years ago

Yay, now I can move along to actually developing what I'm developing. Thanks for this!