search

PolymerLabs / polydev

Automatic web components profiling in chrome devtools
123 stars 10 forks source link

Issues with webcomoonents#v1 and polymer#2.0-preview #39

Closed valdrinkoshi closed 7 years ago

valdrinkoshi commented 7 years ago

I face issues when the chrome extension is enabled and I use the branches webcomoonents#v1 and polymer#2.0-preview in my webpage, I get a bunch of errors dom-module.html:32 Uncaught TypeError: Cannot read property 'register' of undefined(…) http://output.jsbin.com/wuxezi/

rictic commented 7 years ago

Ok, part of the issue was just that I wasn't binding to this for CE callbacks. That was easy to fix.

Harder to fix is the fact that the strategy for measuring v1 custom elements is to wrap the element class in a child class and actually register that. This is of course observable in subtle ways, their constructors aren't the user class, but the element instances are still instanceof the user's class, and in general that would be good enough I'd think…

Except. Except for this:

class Minimal extends HTMLElement {}
customElements.define('minimal-v1', Minimal);

new Minimal();

That's an illegal constructor call, because the Minimal class hasn't been registered as a custom element, a subclass of Minimal has been.

So. What can we do? Well, for everything but the constructor, we could patch the prototype of the class like we do for v0 elements. For the constructor though, there's not an obvious way to patch it, because the user has the original reference and is likely to use it. We could register the user's constructor as a different tagname, but that would just break things in more subtle ways.

In polyfilled environments we could probably hack something together, but it's not clear what we can do in native environments.

Thoughts? /cc @justinfagnani

justinfagnani commented 7 years ago

Yeah, I was wondering about this very issue in reviewing the code. I think you basically have to use the polyfill's native shim, which patches HTMLElement so that new Minimal() ultimately returns an instance of the class you generated and registered. This has a pretty unfortunate ~10% perf hit.

/cc @tjsavage @dominiccooney @paulirish we really need this in dev tools proper.

rictic commented 7 years ago

I think it's probably better to just not measure element creation time. My thinking is that this isn't as bad as seems, as v1 custom elements are very limited in what they can do in their constructors, so we shouldn't be leaving much unmeasured.

justinfagnani commented 7 years ago

They're not as limited as you might think. While elements are barred from setting attributes on themselves or adding children in constructors, they are encouraged in the spec to create and initialize shadow roots, which could be the most expensive thing an element ever does.