Seed: Rendering bug

[flosse]

To reproduce the bug I created a little example:

https://github.com/flosse/seed/tree/rendering-bug

If you click on a menu item the ordering gets destroyed :(

[David-OConnor]

Appears to be recently-introduced. Not sure the cause yet; noticed myself but haven't troubleshooted. Could be related to the new text-node handling/the way the vdom diffs it.

[David-OConnor]

Fixed in latest commit. Related to the ranking system used to determine which child is the correct one, and the new text node system.

[flosse]

... it's still not working in my app :(

[flosse]

steps to reproduce:

• run the menu example
• click first on "Foo" and then on "About"

[David-OConnor]

After further thought, the match/ranking system shouldn't cause bugs like this; it should only affect diffing efficiency. Something else is going on related to patching.

[flosse]

Do you think we can solve this issue today or tomorrow?

[David-OConnor]

Latest commit improves child match system by comparing all sub-children (with subsequently reduced weights for each level), which fixes your example. I'm not convinced the root cause is fixed though, because even if the patch fn matches a child incorrectly, it should alter it until correct.

[flosse]

Yes indeed, the example is working now but still some children in my app (that is a bit more complex) are dancing :(

[David-OConnor]

I think this is something fundamental I overlooked in the diffing algo. It attempts to find the best match in the old vdom, then makes it match the new one. It changes the content, but doesn't resort children. Didn't come up until recently perhaps due to the matching algo working well enough. Latest commit has an attempted, but non-working fix. (towards the bottom of the patch fn).

[flosse]

hm...now the "menu bug" is back again too.
Maybe we should go the other way around: use a boring and slow but stable algorithm + a lot of tests and then optimize it step by step?

[David-OConnor]

Menu bug's back because I commented out parts of the matching system to make it easier to reproduce.

How the diffing works for children: Matching system loops through children of the updated vdom, and tries to find the 'most correct' child node (Which sometimes has a right answer, sometimes doesn't). When it finds it, it patches it recursively to make it correct.

The bug, I think, is due to reordering never happening. If the match system guesses correctly, this never comes up. If it doesn't, we need a reorder. We can't expect the match system to always be right, so need to fix this. I'm attempting a fix using the web_sys::Element::after_with_node_1() method to reorder, but running into borrow-checker issues.

I think a "slow but stable" alternative would be to iterate through the children vec without trying to find a 'best match', updating as we go; this would prevent ordering issues, but if elements are inserted between others, would cause unnecessary DOM updates.

Absolutely need tests to catch things like this, but I'm struggling with wasm-pack's test system.

[David-OConnor]

Fixed / in latest release. Switched to a simpler children-diffing algo that just goes in order instead of trying to find the best match.

[flosse]

I just tried it. The menu looks stable now but one part of the app is still "dancing".
I'll try to create an example where you can see the effect.

[flosse]

ok, here are the steps to reproduce:

1. checkout the menu example in https://github.com/flosse/seed/tree/rendering-bug
2. click on Foo => the h2 is rendered as the last child that is wrong!
3. click on About => there is only one child so everything is ok
4. click on Foo again => now the h2 is on the top as expected
5. click on Home => content 0 is the last child instead of the first!
6. click on Foo again => now the h2 is on the bottom again

[David-OConnor]

Try now. Changed patch from delete old / append new to replace.

[flosse]

...I just started porting my app to another framework but your last patch really solved the problem :) Awesome!! :+1: Thx!