WebReflection / hyperHTML

A Fast & Light Virtual DOM Alternative
ISC License
3.07k stars 112 forks source link

Regression #129 #135

Closed kaste closed 6 years ago

kaste commented 6 years ago

New version 1.12.2 broken for Custom Elements. Same CodePen I used for #129 but with updated hyperHTML package.

https://codepen.io/anon/pen/vWBwOx?editors=1010

(Sorry, no time for details. You probably know your way around anyway.)

cary-smith commented 6 years ago

Not sure if this is related, but the Temperature Calculator example also no longer functions in 1.12.2. I have not dug through the finer details yet but it looks like a null value is passed to the createPath function.

WebReflection commented 6 years ago

thanks for filing this. I've reverted general custom elements RegExp and kept only a-frame one around for now.

@cary-smith please confirm with latest everything is fine (also please link me the example you are looking at, thanks)

WebReflection commented 6 years ago

P.S. apologies a patch should never break, indeed it didn't in my tests. I am investigating what's going on with the change I've made since it should not affect working code at all.

cary-smith commented 6 years ago

@WebReflection - The example works as before. Thanks for fast response!

WebReflection commented 6 years ago

let's see how this goes: https://bugs.chromium.org/p/chromium/issues/detail?id=779165

kaste commented 6 years ago

I wonder how they solved this in Polymer 1 bc they used templates then. Aren't v0 element upgrades async?

WebReflection commented 6 years ago

AFAIK Polymer never used V0 while AFrame use my polyfill which brings V0 in every browser but Chrome because it ships V0. My poly with other browsers work, only native Chrome V0 has issues

kaste commented 6 years ago

Pretty sure I used it. Isn't this what you want? Maybe I'm confused.

    var proto = Object.create(HTMLElement.prototype)

    proto.createdCallback = function() {
      console.log('createdCallback')
    }
    proto.attachedCallback = function() {
      console.log('attachedCallback')
    }

    var XFoo = document.registerElement('x-foo', { prototype: proto })

    // create one via template
    var template = document.createElement('template')
    template.innerHTML = '<x-foo></x-foo>'

    // move the element to a live node
    document.body.append(document.importNode(template.content, true))

    assert.equal(document.body.lastElementChild.constructor, XFoo)
WebReflection commented 6 years ago

I might use importNode but it shouldn’t be needed in general

kaste commented 6 years ago

I think, actually you MUST use importNode. Try it. It's the 'right' thing for template elements. And you can remove the ugly regex.

kaste commented 6 years ago

It's actually specced as such :shrig: https://www.w3.org/TR/2016/WD-custom-elements-20160226/#creating-and-passing-registries

WebReflection commented 6 years ago

Custom Elements V1 work out of the box.

Like I’ve said , I’ll have a look.

Not now though, now I’m a bit busy.

kaste commented 6 years ago

T.i. when you do a redundant

template.ownerDocument.importNode(template.content, true)

you get no upgrades. So you actually have to import the nodes into the current document.

kaste commented 6 years ago

With V1 you register against the window, with v0 you register on document. Literally document.registerElement.

WebReflection commented 6 years ago

You register via customElements and its registry which is same used by V0 in Chrome (or they check each other).

Anyway , I will have a look.

Not today

WebReflection commented 6 years ago

Also, with your logic, I register on the document and with same document I create template and content so this looks just bad V0 behaviour . Point is, I don’t care . Whatever works best, really

kaste commented 6 years ago

It's a different document::

image

WebReflection commented 6 years ago

Live Custom Elements V0 tenst here: https://codepen.io/WebReflection/pen/xXMxEO

Anyway, this is still broken in various ways. I am still trying to figure out how to fix V0 only components which are the only problematic.

WebReflection commented 6 years ago

So, if I use importNode each time I have these pros:

and these cons:

To fix this, I need quite some huge refactoring which I believe is not worth in V1 or it will break your same Code Pen

V2 it is then, ugly hack for A-Frame elements only will stay.

Keeping this open until it's properly fixed in V2

WebReflection commented 6 years ago

closing this and will address the issue in V2