Fix created() {} behavior cross browser.

WebReflection commented 6 years ago

The current created() {...} method should grant somehow that a Custom Element is live and/or well known.

Unfortunately this is inconsistent due bugs present in 2 browsers out of 3 implementing Custom Elements.

Apparently the correct behavior is the one that would see created() invoke fail if the custom element is defined before DOMContentLoaded happens.

https://bugs.chromium.org/p/chromium/issues/detail?id=821831

Workaround

The proposed work around comes from this @cramforce tweet and it seems to work in general but it has the following outstanding issue: in a case where the custom element is the last child of a tree of single/last child it fails to ever return true if the parent node has no white spaces or extra nodes.

This is particularly easy to reproduce via hyperHTML template literals so that such work around might result into a never ready component.

<!doctype html>
<script>
this.onload = function () {
  // define custom-app with the ready work around
  hyper(document.body)`<custom-app say="hello world" />`;
  console.log(document.body.firstChild.ready); // false
};
</script>

In above example document.body as well as document.documentElement and document itself will not have a nextSibling so the result of a main custom element as body app would be "never ready".

Possible improvements

The not fully ready case never really happens with hyperHTML itself and it might occur for custom elements only when defined before DOMContentLoaded with Custom Elements within the body.

This is a specific issue with HyperHTMLElement class only, and very specific to the current document parsing state.

In order to be sure everything works:

explicitly ask developers to define their custom elements in a deferred script
explicitly ask developers to use DOMContentLoaded or the ready pattern
use DOMContentLoaded internally only if document.readyState is not complete keeping same logic in place, queuing all created callbacks invoked 'till that time instead of executing them once

In latter case this .ready getter would be redundant / pointless, since any CE could define this.ready = true in their created constructor.

cramforce commented 6 years ago

AMP short circuits the loop if document.readyState == 'complete' and if the loop determines that there is no nextSibling it does these 2 things

Poll with setInterval(…, 5) for a nextSibling to appear
Or DOMContentLoaded fires.

This could probably also be implemented with MutationObserver but the polling works pretty well.

coveralls commented 6 years ago

Coverage remained the same at 100.0% when pulling 2ccdeafb8801b29470614bee86d7e16173b89582 on ready into 7386ddf0a7d1710684a0b376c6a9eb33033a1072 on master.

WebReflection commented 6 years ago

Thanks @cramforce . My current resolution with full code coverage uses indeed a similar approach.

It's not optimal when a node isn't "ready" for long time but it grants the following:

best case scenario: document.readyState is already complete and created() kicks in before anything else
Chrome case scenario: DOMContentLoaded hasn't fired yet and the node is streamed/not fully know yet. polls via setTimeout
worst case scenario: DOMContentLoaded hasn't fired. attributes are changed. connected callback fired too. The node in stream is taking forever to finalize ... setTimeout and two methods spread various calls to the same entry point but no side effect is caused as soon as the CE is ready or DCL fired.

There's also a guard against fake DOMContentLoaded events so every bloody case should be covered.

Thanks for hints and info.

WebReflection / hyperHTML-Element

Fix created() {} behavior cross browser. #23

Workaround

Possible improvements