-
Notifications
You must be signed in to change notification settings - Fork 67
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
Use morphdom to apply dom changes in dyn nodes #116
base: master
Are you sure you want to change the base?
Conversation
Is there any risk that this could break existing code, or is it purely a rendering optimisation (or any code that broke as a result of this change is probably doing something really dodgy)? The prospect of fast virtual dom-diffing in Ur/web is pretty exciting! |
I just want to make sure it's clear that Ur/Web avoids the need to "diff" trees to avoid many unnecessary regenerations. Different |
Among things like less deep nesting, comments don't interfere with CSS features like CSS Grid
Regarding #113, I used morphdom to update dom changes in dyn nodes differently. Before this change, on every render of a dyn node we removed all dom nodes from the previous render and inserted all new nodes every time. This had some annoying effects, as described in the manual on page 47. The two coping strategies mentioned there are unnecessary after this change. After this change, dom updates will be done more intelligently, diffing the dom nodes from the previous render against the dom nodes of the current render (note: no virtual dom is used) and only applying the necessary dom updates incrementally.
I split the change into three commits for easier comparison.
The first commit vendors morphdom.js as is into urweb.js. I'm not sure if that's the best way but it made it easy to get started.
The second commit adds skipChildren and skipChildrenEnd options to morphdom. These are needed because morphdom takes one "before" node and one "after" node and changes one into the other. This is not how we want to do things when we add trs into tables or tds into trs, so we need to be able to say to morph dom "Here are two nodes, compare their children but skip the first x and last x children in the 'from' node".
The third commit then changes the recreate function to use morphdom for dom updating. I simplified the code a little in the process. Basically in the general case we add a span on the first render in which we render the html string. Next renders we make a new span every time, but the new html string in there, and then let morphdom change the first span to look like the new one. In the table/tr case we add a marker script on the first render, then render all the children in between that marker script and the script in the x variable, using morphdom to work out the differences. (As an added bonus, it seems to handle rendering tbody's a little better.)
Performance-wise I've found it hard to create and capture good benchmarks. In most cases where you're only rendering small amounts of nodes inside a dyn tag, the performance difference will be negligible. The more nodes that stay the same, the more performant this change is: We're creating the same amount of nodes, but only adding and removing what we need, which gives a good performance boost. For a simple benchmark where you can easily see the difference, make a page where you render 10000 elements in a dyn on every tick. The DOM has to do a ton more work before this change.
Phew, I hope this is OK. If you want me to do anything else or different, let me know. I know I've gone and made a PR without waiting for a reply from @achlipala in #113, but I was motivated to make this work. If you don't want to accept this, no hard feelings of course. But I do think this is a good change.
Further optimizations would be cutting out some code from the vendored morphdom code, since we're not using all of it's features. Also the code has quite a few comments etc so adds to the byte size, but I feel that's an orthogonal discussion that maybe belongs in #115 in the discussion about js code minification.