Skip to content
This repository has been archived by the owner on Jul 28, 2023. It is now read-only.

setTimeout callback doesn't seem 100 % reliable #69

Closed
SantosGuillamot opened this issue Sep 16, 2022 · 9 comments
Closed

setTimeout callback doesn't seem 100 % reliable #69

SantosGuillamot opened this issue Sep 16, 2022 · 9 comments
Labels

Comments

@SantosGuillamot
Copy link
Contributor

As discussed here, it looks like the setTimeout callback we are using inside connectedCallback is not 100% reliable. It seems to be running before the HTML parsing has finished. Here you have one example of when this is a problem:

It seems that if the page is loaded quickly, the state defined in PHP and sent as an attribute is undefined in the view.js files. These are the two examples I'm talking about: Title & Date.

As Luis shared here, Jake Archibald talks about this problem in this video. He proposes:

  • A weird custom element hack that can inform the parent when it's rendered, because at that moment, all the children should be rendered as well:
<wp-block>
  <!-- children is processed first -->
  children...
  <!-- then this final element is processed -->
  <children-finished-processing></children-finished-processing>
</wp-block>
  • Use mutation observers.
@luisherranz luisherranz changed the title setTimeout callback doesn't seem 100 % reliable 🏝 setTimeout callback doesn't seem 100 % reliable Sep 19, 2022
@luisherranz luisherranz changed the title 🏝 setTimeout callback doesn't seem 100 % reliable setTimeout callback doesn't seem 100 % reliable Sep 19, 2022
@luisherranz luisherranz added the bug Something isn't working label Sep 19, 2022
@luisherranz
Copy link
Member

@senadir and I have been looking more deeply into this, and there doesn't seem to be a way to solve this using the mutation observer alone. So unless there's a new idea, we would need to go with the element hack described above.

@senadir suggested that instead of a custom element, we could use a combination of a CSS-neutral element (like template, script, style...) to avoid messing with the CSS, and a mutation observer to detect when it appears in the DOM.

@senadir
Copy link

senadir commented Oct 10, 2022

HTML comments are picked up by MutationObserver and have no drawbacks. We do run the risk of optimization plugins removing comments before page render https://wordpress.org/support/topic/strip-html-comments-is-it-safe/

In my testing, it seems that MutationObserver does indeed pick up a template tag
image

@senadir
Copy link

senadir commented Oct 10, 2022

While setTimeout isn't ideal because it just defers the work to the next possible task, you can possibly try using requestIdleCallback which should defer to when the browser is idle, this means when the browser is done parsing data.

@tomalec
Copy link

tomalec commented Nov 4, 2022

it looks like the setTimeout callback we are using inside connectedCallback is not 100% reliable

I'd be happy to know the rationale behind the setTimoeut implementation. What was the goal there?

First of all using connectedCallback there is no guarantee that the element is actually connected :)
See the note at https://developer.mozilla.org/en-US/docs/Web/Web_Components/Using_custom_elements#using_the_lifecycle_callbacks

Note: connectedCallback may be called once your element is no longer connected, use Node.isConnected to make sure.

Also, from a custom element life-cycle perspective, connectedCallback is also not the right moment to collect attribute values - that should be done in attributeChangedCallback.

connectedCallback is there to be called when the element itself was connected, and has nothing to do with it's children.

@luisherranz
Copy link
Member

I'd be happy to know the rationale behind the setTimoeut implementation. What was the goal there?

We need a callback that triggers once the custom element's children have been created because they are required for the hydration (to extract sourced attribute, use hydrate instead of render, etc.).

@tomalec
Copy link

tomalec commented Nov 5, 2022

We need a callback that triggers once the custom element's children have been created because they are required for the hydration (to extract sourced attribute, use hydrate instead of render, etc.).

Given this is an element in the live document. So in DOM "Have been created" is not an event that happens only once, but may happen later in runtime. New children elements could be added, and old ones could be removed.

So to be precise on what we need, I guess "created" is also not the thing we need, as children may be created far before or wp-block is created. I guess we need the moment when they are connected to wp-block subtree.

When you write "sourced attribute" what do you mean by "sourced" and that's the attribute of what?

Please correct me if I'm wrong here because it's all based on suspicions ;) And I don't know the lingo you use in this project, but rather stick to HTML definitions. What we need here is that:

Run a piece of code:

  1. Only once
  2. only after initial parsing of the document (/subtree/.innerHTML)
  3. only after the parser reaches the end of our element/subtree/document fragment
  4. ignore all the subsequent changes/mutations to that subtree

So if the above is what we want, for sure conectedCallback() { setTimeout has no chance to be reliable.


If we really want to achieve "parser reaches the end of our element" then the problem is HTML parser does expose any callback for parsing the end tag. AFAIR this is due to how the HTML parser is implemented deep in browser engines.

The only element that somewhat has that feature is the <template> element.

That is precisely the reason why Declarative Shadow DOM eventually ended up being specified & implemented as <template shadowroot>.

BTW, DSD was created among others to support Server Side Rendering. So I think it's worth looking for inspiration, especially since most of the points I mentioned above, were also discussed and covered there.

@tomalec
Copy link

tomalec commented Nov 5, 2022

Also, as I usually like to step back from the solution, to define a problem, and find a new out without a bias of having one in mind:

Do we really need to do it "once the custom element's children have been created"? If so, why? Maybe we can hydrate the elements as they come?

@luisherranz
Copy link
Member

When you write "sourced attribute" what do you mean by "sourced" and that's the attribute of what?

Sourcing attributes is a common way to store block attributes in the HTML to avoid duplication of data and sync issues: documentation.

Do we really need to do it "once the custom element's children have been created"? If so, why?

Yes. Before we can hydrate the block, we need to get the sourced attributes, whose values are stored somewhere in the children instead of the JSON (block comment) and we also need all the children already created to be able to call React/Preact's hydrate function and avoid the unnecessary recreation of DOM nodes: documentation.


By the way, this issue is only relevant if we finally use a custom element to do the hydration, which is still not clear. Let's continue that conversation on this issue instead.

@luisherranz
Copy link
Member

Closed as we're not actively working on this experiment anymore.

@luisherranz luisherranz closed this as not planned Won't fix, can't repro, duplicate, stale Jan 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants