Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

order of patched/mounted across renderings may not be what we want #1445

Open
ged-odoo opened this issue May 30, 2023 · 1 comment
Open

order of patched/mounted across renderings may not be what we want #1445

ged-odoo opened this issue May 30, 2023 · 1 comment
Labels
enhancement New feature or request

Comments

@ged-odoo
Copy link
Contributor

Owl guarantees that in a render:

  • all onWillPatch hooks are called (in the parent->child order) before the DOM is modified
  • then we patch the DOM with the new virtual dom
  • finally, we call onMounted/onPatched hooks are called (in the child->parent) order

The idea is that we may want to measure stuff in onWillPatch before the dom is changed, so it should be called before. Also, once it's done, we want to call the onMounted and onPatched in the opposite order, so a parent is guaranteed that all its children are ready.

This works fine, as far as I can tell. However, what if we have multiple renderings?

For example, imagine that we have two components: Parent and Child, with no props, so the Child can be updated independently. Now, we update Parent, and a few micro ticks later, we update Child. See playground

So, if you look at it, you will see that clicking on the update button logs the following:

parent:onWillPatch
parent:onPatched
child:onWillPatch
child:onPatched

This is because we have two distincts renderings, and they are independent. So, even though they complete in the same animation frame, they are not properly coordinated.

This means that the parent onPatched hook is called before the child, which may be an issue (this is why I was notified of this problem).

We can probably easily reorder the fibers before applying them, but then we would have the following:

child:onWillPatch
child:onPatched
parent:onWillPatch
parent:onPatched

The onPatched hook are correct, but then the parent onWillPatch is called after patching its child, so it seems wrong also.

It feels like we actually want this:

parent:onWillPatch
child:onWillPatch
child:onPatched
parent:onPatched

But this would interleave actions taken from two different renderings, which is a big change.

@ged-odoo
Copy link
Contributor Author

Note that the problem was always present, but may be more pressing now if we use the fine grained reactivity, because there are now many more situations where we have small independent renderings

@sdegueldre sdegueldre added the enhancement New feature or request label Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants