bikeshaving / crank

The Just JavaScript Framework
https://crank.js.org
MIT License
2.7k stars 75 forks source link

Experiment with creating/patching nodes in a pre-order traversal of the element tree #176

Closed brainkim closed 3 years ago

brainkim commented 3 years ago

Currently, Crank performs all DOM mutations in a post-order traversal of the tree. However, now that we’ve split out the different types of mutations into create/patch/arrange, we can now consider moving the creation and patching of nodes to a pre-order traversal of the tree. The motivation for trying this out is that I’ve been noticing that sometimes the arrangement of previously rendered children will invalidate layout, and then patch at a higher level of the tree will cause blocking layout shifts. I don’t know if this will be faster or slower, or even stop the layout shift issue, but it’s something to experiment with. Intuitively, setting the properties of DOM nodes before they have children might be faster? Who knows.

brainkim commented 3 years ago

This seems to improve performance slightly on the big JS frameworks benchmark, but I’m not sure if it’s statistically significant, and the tests don’t actually trigger the layout shifts I’m seeing in my current application, where layout is invalidated somewhere lower in the tree and then requested higher in the tree. Nevertheless, I am okay with this change even if it means only equivalent performance on the big benchmark.

I’m thinking about this some more, and realizing it’s also a slight breaking change in the sense that rendered DOM nodes will be updated before async children resolve. I don’t think this is a huge issue, and in many cases this behavior seems more desirable, the reasoning being if you wanted some DOM changes to happen asynchronously, you would put it below the asynchronous component which you wanted it to happen after.

brainkim commented 3 years ago

Implemented in 0.3.8. Leaving this issue as a reminder to update custom renderer documentation.

brainkim commented 3 years ago

As I’m implementing async scheduling, I’m thinking about reverting this change. Being able to defer not just arranging but patching seems like a useful feature, and the performance advantages of doing patching earlier, if there are any, don’t seem to outweigh this ability.

Realizing that nothing about async component updating could cause patching of elements to be deferred insofar as the children under the component will always render to completion anyways, and component elements do not use their props to update the DOM directly. Never mind, though I still maybe want to revert the change I dunno.