Children are rendered out of order sometimes (test case included)

[zimond]

Problem
I found that sometimes yew renders children out of order. After several days of separating the behavior, I managed to create a small repo to reproduce this: https://github.com/zimond/yew_bug

Steps To Reproduce
Steps to reproduce the behavior:

1. Clone the repo, run npm run start
2. In the page, click "3", then click "0"
3. Notice that the div above the h1 element is now under the element.

The required code to trigger this behavior is quite tricky. I found that you need to:

1. in wrapper.rs#51, this blank {html!{}} is a must, or the disorder doesn't show up
2. in wrapper.rs, a message need to be dispatched to trigger an update, or the disorder doesn't show up

Expected behavior
The div should always be above the header

Screenshots
Before you click:

After clicking "3" & "0"

Environment:

• Yew versionv 0.14.1
• Rust version rustc 1.43.0-nightly (6fd8798f4 2020-02-25)
• Target if relevant : wasm32-unknown-unknown]
• stdweb / web-sys : web_sys
• OS: macos
• Browser [e.g. chrome, safari]
• Browser version [e.g. 22]

Questionnaire

• I'm interested in fixing this myself but don't know where to start
• I would like to fix and I have a solution
• I don't have time to fix this right now, but maybe later

[jstarry]

Thank you so much for the report and testcase! I won't have time for a few days to look into this, @TheNeikos are you interested in taking a look?

[TheNeikos]

I'm not too familiar with Yew yet, but it's a good opportunity to try!

[jstarry]

Sweet, feel free to ask questions here!

[TheNeikos]

Alright, so I think I'm getting to the bottom of this:

<VList as VDiff>::apply tries to render a list of [Element, VList] having an ancestor of [Element, Vlist].

This in turn, calls <VComp as VDiff>::apply with the other Element as ancestor. However, it sees that the components are mismatched and removes the old element! This is an issue, as we have now lost our positional anchor! In the next steps it then tries to find a sibling, but fails to do so and thus resorts to append the component to the end.

This is why the text "above header" is now rendered below the header.
The fix is then to remember the position of the old component, and insert the component there. I'm currently getting that to work!

---

Edit: Upon further debugging, this is not the complete picture. Shame!

[TheNeikos]

@zimond At first your test-case was working, but right now I am unable to reproduce it. Can you check if this is still an issue for you? I am not sure how this could have 'fixed' itself as I reset everything. (The only thing I can imagine is a browser update in the background? But not sure how I could confirm that)

[zimond]

@TheNeikos I just ran cargo clean and recompiled and it is still happening. To clarify you need to exactly click "3" then back to "0", clicking other tabs will reset the ordering and cannot trigger the behavior

About the browser, I tested firefox nightly and chrome, the bug consists.

[TheNeikos]

Alright, then something weird is going on over here. I'll keep investigating

[captain-yossarian]

@TheNeikos Hi, if you add new child to

        <Tabs>
            <div>
                <Card title="above header"/>
                <h1>{"tab 1"}</h1>
            </div>
            <div>{"dumb 1"}</div>
            <div>{"dumb 2"}</div>
            
            <div>
                <Custom content="tab 2"/>
                {html!{}}              
            </div> 

          // new child
            <div> 
                <div>{4}</div>
                <div>{4.1}</div>
                {html!{}}
            </div>

        </Tabs>

The code will work as expected, because new child does not include Component instance.
It looks like Component instance affects the logic.
Which data structure represents yew Component ? stdweb::web::{Element} ?

I think the problem might be here.
@jstarry what do you think?

[TheNeikos]

Yeah, I tried a few things. But from my debugging until I wasn't able to reproduce the error again, I was unable to pinpoint where things are going wrong. What I found was that components are rendered in a second pass, after other more simpler nodes are rendered. And it seems that the node in that moment then gets lost and things get appended at the end. But I can't confirm this, as wasm is hard to debug. No stepping etc..

[captain-yossarian]

@TheNeikos to debug source code of yew, you can add

use cfg_match::cfg_match;

cfg_if! {
    if #[cfg(feature = "std_web")] {
        #[allow(unused_imports)]
        use stdweb::{_js_impl, js};
    } 
}

pub fn log(message: &str) {
    cfg_match! {
        feature = "std_web" => js! { @(no_return) console.log(@{message}); },
    };
}

and then somewhere in the code just add:

log("your debug string")

[zimond]

wasm-logger is an out of box solution for logging.

[TheNeikos]

Yeah, logging is working fine luckily! But to understand how the algorithm works and which branches it takes etc... especially with all the indirection of traits I prefer stepping through, otherwise it does make it a bit harder to see the flow.

[captain-yossarian]

@TheNeikos I spent some time on this bug and I also don't know how it works:)

[jstarry]

Thanks for investigating! I'll take a look and see if I can give some pointers

[mrh0057]

@jstarry and @TheNeikos I was able to trace it back to here. It looks like it's an issue since they are 2 different components being rendering. Still looking at it to see if I can figure out how to fix the issue.

[jstarry]

Interesting, maybe a regression caused by #983

[mrh0057]

@jstarry The noderefs are none on the VComp are none. I checked mounted and the vcomp node field. Looking to see if I can trace back where they are being removed.

[mrh0057]

@jstarry Looks llike the NodeRef is returning None in Mounted and on the VComp for the ancestor node. It is correct in the scope and shows up when calling destroy. Haven't been able to figure out why it's not set in Mounted nor on the VComp.

[mrh0057]

@jstarry I think it has something to do with the children and having the first value a VComp. I added logging statements outputting the strong count of the NodeRef and it looks like the scope has a different NodeRef since the strong_count is only 1 when it gets destroyed and it has a value set for Node. The strong count for the VComp and Mounted is 4 on the ancestor when it tries to do the update and the NodeRef isn't set.

[jstarry]

@mrh0057 you were right on the 💰

The issue is that node refs were not set properly when components are updated 😬

The node_ref set here is never set or linked

[jstarry]

Power of open source collaboration 💪

[TheNeikos]

@mrh0057 Ha! I knew that part felt wrong, I didn't think of the Rc though. Thanks for solving this and giving me some peace of mind!