Preact: useLayoutEffect teardown runs after new elements are mounted

[d3x7r0]

After moving to Preact X and using hooks I started noticing something weird happening with the useLayoutEffect hook that I was using to store the scroll position when exiting a route.

What I found is that when the teardown function of the hook runs the DOM contains both the old and the new children at the same time (hence my scroll position was being read wrongly).

Here's an example:
https://codesandbox.io/s/zealous-snyder-37ync

When clicking next the component should change the visible component and there's a console.log of the DOM when the hook runs. You can see that when running the teardown function both the old and the new children are mounted in the DOM.

I'm not sure if this intended behaviour and I'm just using the hook wrong of if it's working as intended.

EDIT: for reference here is the same thing in react where the problem doesn't show up.
https://codesandbox.io/embed/nostalgic-taussig-m09gx

[d3x7r0]

I looked into the source code a little bit and seems to me like the problem shows up because layout hooks run on the options.diffed callback which means that will run at multiple times during the diffing process (so Foo might still be present in the DOM because the diffing function hasn't reached it yet).

Seems to me like the code should instead run when commitRoot runs instead but for that there needs to be some way to collect all nodes that need to be affected. It should be possible to collect them on the existing diffed callback and then flush them on the commit one but I can't see a way to make sure we don't mix up different roots (I'm not too familiar with the layout of the vnodes).

I would be happy to take a stab at a solution if someone can help me out with some knowledge of the internals of preact (like how a vnode is structured for example) and some ideas on how to work around this issue.

[JoviDeCroock]

The problem you are talking about is that it would need a two phase render cycle, this would mean that all vnodes are diffed, and written afterwards that would make the accurate reading possible. At this point we are using a one phase render cycle and the two phase is planned After X releases.

[d3x7r0]

Oh I see. Fair enough. I also just noticed that the effect and cleanup functions will probably need to be handled separately since the later one will need to run before DOM removals happen and the former after. So yeah, this might be a bit trickier then it seemed at first glance. :)

Anyway if I can be of any help let me know.

[JoviDeCroock]

Indeed it should be:

diffing --> results in knowing dom structure
cleanup layout effects
handleDomChanges
trigger layout effects

[andrewiggins]

Hey @d3x7r0, if you have time, could you try your original scroll position use case with #2012? I think it'll resolve the scroll position problem. Or if you have simple repro I can try it

[d3x7r0]

Hi there. I do still have that codesandbox I pointed out above but I can't seem to get it to work when linking preact using npm. I'll keep trying but if anyone wants to try it as well it's just a single js file.

[d3x7r0]

Ok I managed to get it working here and I have bad news: the behavior seems to be the same. When clicking the button the code still sees both the old and new elements at the same time on the dom. 😔

[andrewiggins]

Thanks for trying it out! I looked it at and, yes, it seems your bug is a new case I hadn't considered. @JoviDeCroock created a UT for us based on your bug and I added it to my PR so we can play with it. We may check in #2012 as is to fix other issues but we'll definitely add this as a test case for us to work on in the meantime.