<For/> ignores the order of items

[takeisit]

Hi, the following code renders a list of numbers.

#[component]
fn ForTest(cx: Scope) -> impl IntoView
{
    let (list, set_list) = create_signal(cx, vec![1]);

    view!{cx,
        <ul>
            <For each=list
                 key=|item|*item
                 view=|cx,item|view!{cx, <li>{item}</li>} />
            <button on:click=move|_|set_list(vec![0,1,2])>
                "update to 0 1 2"
            </button>
        </ul>
    }
}

Expected Behavior for me:
1 -> push the button -> 0 1 2

Actual Behavior:
1 -> push the button -> 1 0 2

version: 0.2.0-alpha
feature: csr (default)

[ohyjek]

I was testing this a bit today and it also doesn't appear to accept duplicates in the new vec either i.e.

#[component]
pub fn ForTest(cx: Scope) -> impl IntoView
{
    let (list, set_list) = create_signal(cx, vec![1]);

    view!{cx,
        <ul>
            <button on:click=move|_| {
                let new_vec = vec![0, 1, 2, 2];
                set_list(new_vec);
            }> 
            "Test" 
            </button>
            <For each=list
                 key=|item|*item
                 view=move |item| {
                    view!{cx,             
                        <li>{item}</li>}
                } 
            />
        </ul>
    }
}

Expected Behavior for me:
1 -> push the button -> 0 1 2 2

Actual Behavior:
1 -> push the button -> 1 0 2

version: 0.2.0-alpha
feature: csr (default)

[ohyjek]

Interestingly

let (list, set_list) = create_signal(cx, vec![-1]); // note the start is now -1 (a different value than what we will set)

And

let new_vec = vec![0, 1, 2, 2]; // 1 isn't in the initial vector so this will be  [0, 1, 2, 2,] -> [0, 1, 2]
set_list(new_vec);

This will now have the correct order but won't have the right amount of elements.

Expected Behavior for me:
1 -> push the button -> 0 1 2 2

Actual Behavior:
1 -> push the button -> 0 1 2

version: 0.2.0-alpha
feature: csr (default)

[ohyjek]

Ah okay so here's the issue...

It's the key...

Basically

key=|item|*item

Is a bad idea because we need the key prop to be unique, and if the type of item is just &i32, there is no guarantee of uniqueness. @takeisit

#[component]
pub fn ForTest(cx: Scope) -> impl IntoView
{
    let (list, set_list) = create_signal(cx, vec![1]);

    view!{cx,
        <ul>
            <button on:click=move|_| {
                let new_vec = vec![0, 1, 2, 2];
                // set_list.update(|vec: & mut Vec<i32>| {
                //     *vec = vec![2, 0, 1, 2, 2];
                // });
                set_list(new_vec);
            }> 
            "Test" 
            </button>
            <For each=list
                 key=|_| Uuid::new_v4() // notice that we now generate a unique ID
                 view=move |item| {
                    view!{cx,             
                        <li>{item}</li>}
                } 
            />
        </ul>
    }
}

Will render as expected

Expected Behavior for me:
1 -> push the button -> 0 1 2 2

Actual Behavior:
1 -> push the button -> 0 1 2 2

✅

version: 0.2.0-alpha
feature: csr (default)

[takeisit]

I'm sorry for the lack of explanation.
My code above is a minimum code to reproduce.

• The actual type of key in my projcet is ULID instead of i32, and the type of item is a struct which have the key.
• I want to insert new items before and after an existing item in a list.

In my observation using the below code, it seems to occur when multiple items are inserted to multiple separated places in one update.

#[component]
fn ForTest2(cx: Scope) -> impl IntoView
{
    let (list, set_list) = create_signal(cx, vec![]);
    let (text, set_text) = create_signal(cx, "".to_owned());
    let (deduped_text, set_deduped_text) = create_signal(cx, "".to_owned());
    let (prev_text, set_prev_text) = create_signal(cx, "".to_owned());

    let update = move|_| {
        let mut deduped = Vec::new();
        let mut seens = HashSet::new();
        let mut dedup_node = "".to_string();
        for item in text().split_ascii_whitespace() {
            if !item.is_empty() {
                if seens.insert(item.to_owned()) {
                    deduped.push(item.to_owned());
                } else {
                    dedup_node = format!(" (deduped from {})", text());
                }
            }
        }

        let deduped_text = deduped.join(" ") + &dedup_node;

        set_deduped_text(deduped_text);
        set_prev_text(list().join(" "));
        set_list(deduped);
    };

    view!{cx,
        <ul>
            <li>"prev____:" {prev_text}</li>
            <li>"expected:" {deduped_text}</li>
            <li>"actual__:"
                <For each=list
                    key=|item| item.to_owned() // key is unique String here
                    view=|cx,item|view!{cx, <span>{item}{" "}</span>} />
            </li>
            <li>"next___:"
                <input on:change=move|e| set_text(event_target_value(&e)) />
                <button on:click=update>"update"</button>
            </li>
        </ul>
    }
}

• NG: "1 3" -> update to "0 1 2 3", but actual: "1 0 2 3" -> update to "", but actual: "1"
  (reload page to refresh states)
• OK: "1 3" -> update to "0 1 3" -> update to "0 1 2 3" -> update to ""
  (reload page to refresh states)
• OK: "0 3" -> update to "0 1 2 3"
  (reload page to refresh states)
• OK: "2 3" -> update to "0 1 2 3"
  (reload page to refresh states)
• OK: "0 1" -> update to "0 1 2 3"
  (reload page to refresh states)
• NG: "1 2 3" -> update to "a 1 b 2 c 3 d", but actual: "1 2 3 a b c d"

[gbj]

Thanks for the detailed report. We're taking a look. This is clearly at least in part a bug in <For/>, so stay tuned.

@omie-lol, if you generate unique IDs inside the key function, it will generate a unique ID for every row on every iteration, rather than a stable ID across changes. This means every row will be replaced every time, which is almost certainly not what you want. In fact it ends up just being a less-efficient form of plain iteration, like the below.

If it's helpful to unblock your work for now, you can always use a (less-efficient) unkeyed list like this

<ul>
{move || list().into_iter()
  .map(|item| view!{ cx, <span>{item}{" "}</span>})
  .collect::<Vec<_>>()}
</ul>

But we'll dig into it.

[gbj]

Sorry I haven't left an update here in a while. This was an issue specifically involving moving the item at index 1, but it has led @jquesada2016 to work on writing some comprehensive tests for this component and he's in the late stages of an updated algorithm which should resolve this and any similar edge cases.