Fix keyed list ordering issue (#1231)

* New example to showcase keyed elems reordering issue

* Allow building examples in release mode

* Fix ordering issue with keyed virtual nodes

* review comments for build.sh

* review comments for examples/keyed_list

* fix style.css to keyed_list example

* remove ahash

* Add GitHub issue to TODO in key.rs

* Address some review comments in virtual_dom

* New diffing algorithm for keyed vlists

* Add forgotten dependency for yew-stdweb

* Add tests for vlist diffing

* Removed VDiffNodePosition

* Fix usage of next_sibling for vlist

* Fix tests for stdweb

* Mitigate issue with moving VLists when children are all VLists

Note: The new UT is failing, I now. I want to discuss that before
maybe fixing it (moving all VList children?).

* Fix issue with inserting into vlist that is not last child

* Refactor VDiff trait to make way for keyed list fixes

* Fix quote_spanned macro invocations

* Revert some minor changes (style, Debug)

* Revert some minor changes (style, Debug)

* Fix self-referencing NodeRef issue

* All VList tests pass

* Fix algorithm choice in degenerated case

* Remove stdweb and non keyed tests

* Key from finite list of types

* WIP moving VList tests to diff_layouts

* Removed unnecessary Vec

* Fix VComp NodeRef self linking issue

* Add logs to diff_layouts tests

* WIP moving VList tests to diff_layouts

* WIP Failing test moving VComp

* Add VComp move_before

* Fix list component change method

* Fix bad merge

* Add more protection against node ref cycles

* Remove commented tests

* Feedback and clippy

* Failing test

* tests pass

Co-authored-by: Justin Starry <justin.starry@icloud.com>

Cargo.toml

@@ -40,9 +40,10 @@ members = [
   "examples/game_of_life",
   "examples/inner_html",
   "examples/js_callback",
+  "examples/keyed_list",
   "examples/large_table",
-  "examples/minimal",
   "examples/minimal_wp",
+  "examples/minimal",
   "examples/mount_point",
   "examples/multi_thread",
   "examples/nested_list",

examples/build.sh

@@ -1,44 +1,60 @@
 #!/usr/bin/env bash
 
+# The example to build.
+EXAMPLE=${1%\/}
+# Optimization level. Can be either "--debug" or "--release". Defaults to debug.
+PROFILE=${2:---debug}
+
 # src: https://gist.github.com/fbucek/f986da3cc3a9bbbd1573bdcb23fed2e1
 set -e # error -> trap -> exit
-function info() { echo -e "[\033[0;34m $@ \033[0m]"; } # blue: [ info message ]
-function fail() { FAIL="true"; echo -e "[\033[0;31mFAIL\033[0m] $@"; } # red: [FAIL]
-trap 'LASTRES=$?; LAST=$BASH_COMMAND; if [[ LASTRES -ne 0 ]]; then fail "Command: \"$LAST\" exited with exit code: $LASTRES"; elif [ "$FAIL" == "true"  ]; then fail finished with error; else echo -e "[\033[0;32m Finished! Run $@ by serving the generated files in examples/static/ \033[0m]";fi' EXIT
+function info() { echo -e "[\033[0;34m $* \033[0m]"; } # blue: [ info message ]
+function fail() { FAIL="true"; echo -e "[\033[0;31mFAIL\033[0m] $*"; } # red: [FAIL]
+trap 'LASTRES=$?; LAST=$BASH_COMMAND; if [[ LASTRES -ne 0 ]]; then fail "Command: \"$LAST\" exited with exit code: $LASTRES"; elif [ "$FAIL" == "true" ]; then fail finished with error; else echo -e "[\033[0;32m Finished! Run $EXAMPLE by serving the generated files in examples/static/ \033[0m]";fi' EXIT
 SRCDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" # this source dir
 
-cd $SRCDIR # ensure this script can be run from anywhere
+cd "$SRCDIR/$EXAMPLE" # "$SRCDIR" ensures that this script can be run from anywhere.
 
 # When using $CARGO_TARGET_DIR -> binary is located in different folder
 # Necessary to locate build files for wasm-bindgen
-TARGET_DIR=$SRCDIR/../target/wasm32-unknown-unknown/debug
-if [ ! -z "$CARGO_TARGET_DIR" ]; then 
-    TARGET_DIR=$CARGO_TARGET_DIR/wasm32-unknown-unknown/debug
+TARGET_DIR=$SRCDIR/../target/wasm32-unknown-unknown
+if [ -n "$CARGO_TARGET_DIR" ]; then
+    TARGET_DIR=$CARGO_TARGET_DIR/wasm32-unknown-unknown
+fi
+if [[ "$PROFILE" = "--release" ]]; then
+    TARGET_DIR=$TARGET_DIR/release
+else
+    TARGET_DIR=$TARGET_DIR/debug
 fi
 
-EXAMPLE=${1%\/}
-cd $EXAMPLE
+# Build the correct cargo build command depending on the optimization level.
+cargo_build() {
+    if [[ "$PROFILE" = "--release" ]]; then
+        cargo build --release --target wasm32-unknown-unknown "$@"
+    else
+        cargo build --target wasm32-unknown-unknown "$@"
+    fi
+}
 
 # wasm-pack build
-if [[ $EXAMPLE == *_wp ]]; then 
+if [[ $EXAMPLE == *_wp ]]; then
     info "Building: $EXAMPLE using wasm-pack"
     # wasm-pack overwrites .gitignore -> save -> restore
-    cp $SRCDIR/static/.gitignore $SRCDIR/static/.gitignore.copy
-    wasm-pack build --debug --target web --out-name wasm --out-dir $SRCDIR/static/
-    rm $SRCDIR/static/.gitignore; mv $SRCDIR/static/.gitignore.copy $SRCDIR/static/.gitignore # restore .gitignore
+    cp "$SRCDIR/static/.gitignore" "$SRCDIR/static/.gitignore.copy"
+    wasm-pack build "$PROFILE" --target web --out-name wasm --out-dir "$SRCDIR/static/"
+    rm "$SRCDIR/static/.gitignore"; mv "$SRCDIR/static/.gitignore.copy" "$SRCDIR/static/.gitignore" # restore .gitignore
 
 # multi_thread build -> two binary/wasm files
 elif [[ $EXAMPLE == multi_thread ]]; then
     info "Building: $EXAMPLE app using wasm-bindgen"
-    cargo build --target wasm32-unknown-unknown --bin multi_thread_app
-    wasm-bindgen --target web --no-typescript --out-dir $SRCDIR/static/ --out-name wasm $TARGET_DIR/multi_thread_app.wasm
+    cargo_build --bin multi_thread_app
+    wasm-bindgen --target web --no-typescript --out-dir "$SRCDIR/static/" --out-name wasm "$TARGET_DIR/multi_thread_app.wasm"
 
     info "Building: $EXAMPLE worker using wasm-bindgen"
-    cargo build --target wasm32-unknown-unknown --bin multi_thread_worker
-    wasm-bindgen --target no-modules --no-typescript --out-dir $SRCDIR/static/ --out-name worker $TARGET_DIR/multi_thread_worker.wasm
+    cargo_build --bin multi_thread_worker
+    wasm-bindgen --target no-modules --no-typescript --out-dir "$SRCDIR/static/" --out-name worker "$TARGET_DIR/multi_thread_worker.wasm"
 
 else # Default wasm-bindgen build
     info "Building: $EXAMPLE using wasm-bindgen"
-    cargo build --target wasm32-unknown-unknown 
-    wasm-bindgen --target web --no-typescript --out-dir $SRCDIR/static/ --out-name wasm $TARGET_DIR/$EXAMPLE.wasm
+    cargo_build
+    wasm-bindgen --target web --no-typescript --out-dir "$SRCDIR/static/" --out-name wasm "$TARGET_DIR/$EXAMPLE.wasm"
 fi

examples/keyed_list/Cargo.toml

@@ -0,0 +1,13 @@
+[package]
+name = "keyed_list"
+version = "0.1.0"
+authors = ["Thomas Lacroix <toto.rigolo@free.fr>"]
+edition = "2018"
+
+[dependencies]
+log = "0.4"
+rand = { version = "0.7.3", features = ["wasm-bindgen"] }
+instant = { version = "0.1", features = [ "wasm-bindgen" ] }
+wasm-logger = "0.2.0"
+yew = { path = "../../yew" }
+yewtil = { path = "../../yewtil" }

examples/keyed_list/src/lib.rs

@@ -0,0 +1,349 @@
+#![recursion_limit = "1024"]
+
+use instant::Instant;
+use rand::Rng;
+use std::rc::Rc;
+use yew::prelude::*;
+use yewtil::NeqAssign;
+
+pub struct Model {
+    link: ComponentLink<Self>,
+    persons: Vec<PersonType>,
+    last_id: usize,
+    keyed: bool,
+    build_component_ratio: f64,
+}
+
+pub enum Msg {
+    CreatePersons(usize),
+    CreatePersonsPrepend(usize),
+    ChangeRatio(String),
+    DeletePersonById(usize),
+    DeleteEverybody,
+    SwapRandom,
+    ReverseList,
+    SortById,
+    SortByName,
+    SortByAge,
+    SortByAddress,
+    ToggleKeyed,
+    Rendered(Instant),
+}
+
+enum PersonType {
+    Inline(PersonInfo),
+    Component(PersonInfo),
+}
+
+#[derive(PartialEq, Debug, Clone)]
+struct PersonInfo {
+    id: usize,
+    name: Rc<String>,
+    address: Rc<String>,
+    age: usize,
+}
+
+struct PersonComponent {
+    info: PersonInfo,
+}
+
+#[derive(PartialEq, Clone, Properties)]
+struct PersonProps {
+    info: PersonInfo,
+}
+
+impl Component for Model {
+    type Message = Msg;
+    type Properties = ();
+
+    fn create(_: (), link: ComponentLink<Self>) -> Self {
+        Model {
+            link,
+            persons: Vec::with_capacity(200),
+            last_id: 0,
+            keyed: true,
+            build_component_ratio: 0.5,
+        }
+    }
+
+    fn update(&mut self, msg: Self::Message) -> ShouldRender {
+        match msg {
+            Msg::CreatePersons(n) => {
+                for _ in 0..n {
+                    self.last_id += 1;
+                    self.persons.push(PersonType::new_random(
+                        self.last_id,
+                        self.build_component_ratio,
+                    ));
+                }
+                true
+            }
+            Msg::CreatePersonsPrepend(n) => {
+                for _ in 0..n {
+                    self.last_id += 1;
+                    self.persons.insert(
+                        0,
+                        PersonType::new_random(self.last_id, self.build_component_ratio),
+                    );
+                }
+                true
+            }
+            Msg::ChangeRatio(ratio) => {
+                let ratio: f64 = ratio.parse().unwrap_or(0.5);
+                if self.build_component_ratio.neq_assign(ratio) {
+                    log::info!("Ratio changed: {}", ratio);
+                    true
+                } else {
+                    false
+                }
+            }
+            Msg::DeletePersonById(id) => {
+                if let Some(idx) = self.persons.iter().position(|p| p.info().id == id) {
+                    self.persons.remove(idx);
+                    true
+                } else {
+                    false
+                }
+            }
+            Msg::DeleteEverybody => {
+                self.persons.clear();
+                true
+            }
+            Msg::SwapRandom => {
+                let idx_a = rand::thread_rng().gen::<usize>() % self.persons.len();
+                let idx_b = rand::thread_rng().gen::<usize>() % self.persons.len();
+                let id_a = self.persons.get(idx_a).unwrap().info().id;
+                let id_b = self.persons.get(idx_b).unwrap().info().id;
+                log::info!("Swapping {} and {}.", id_a, id_b);
+                self.persons.swap(idx_a, idx_b);
+                true
+            }
+            Msg::ReverseList => {
+                self.persons.reverse();
+                true
+            }
+            Msg::SortById => {
+                self.persons
+                    .sort_unstable_by(|a, b| a.info().id.cmp(&b.info().id));
+                true
+            }
+            Msg::SortByName => {
+                self.persons
+                    .sort_unstable_by(|a, b| a.info().name.cmp(&b.info().name));
+                true
+            }
+            Msg::SortByAge => {
+                self.persons.sort_by_key(|p| p.info().age);
+                true
+            }
+            Msg::SortByAddress => {
+                self.persons
+                    .sort_unstable_by(|a, b| a.info().address.cmp(&b.info().address));
+                true
+            }
+            Msg::ToggleKeyed => {
+                self.keyed = !self.keyed;
+                true
+            }
+            Msg::Rendered(time_before) => {
+                let time_after = Instant::now();
+                let elapsed_max = time_after - time_before;
+                log::info!("Rendering started {} ms ago.", elapsed_max.as_millis());
+                false
+            }
+        }
+    }
+
+    fn change(&mut self, _: Self::Properties) -> ShouldRender {
+        false
+    }
+
+    fn view(&self) -> Html {
+        self.link.send_message(Msg::Rendered(Instant::now()));
+
+        let ids = if self.persons.len() < 20 {
+            self.persons
+                .iter()
+                .map(|p| p.info().id.to_string())
+                .collect::<Vec<_>>()
+                .join(" ")
+        } else {
+            String::from("<too many>")
+        };
+
+        html! {
+            <>
+                <div class="buttons">
+                    <button onclick=self.link.callback(|_| Msg::DeleteEverybody)>
+                        { "Delete everybody" }
+                    </button>
+                    <button onclick=self.link.callback(|_| Msg::CreatePersons(1))>
+                        { "Create 1" }
+                    </button>
+                    <button onclick=self.link.callback(|_| Msg::CreatePersons(5))>
+                        { "Create 5" }
+                    </button>
+                    <button onclick=self.link.callback(|_| Msg::CreatePersons(100))>
+                        { "Create 100" }
+                    </button>
+                    <button onclick=self.link.callback(|_| Msg::CreatePersons(500))>
+                        { "Create 500" }
+                    </button>
+                    <button onclick=self.link.callback(|_| Msg::CreatePersonsPrepend(1))>
+                        { "Prepend 1" }
+                    </button>
+                    <button onclick=self.link.callback(|_| Msg::CreatePersonsPrepend(5))>
+                        { "Prepend 5" }
+                    </button>
+                    <button onclick=self.link.callback(|_| Msg::SwapRandom)>
+                        { "Swap random" }
+                    </button>
+                    <button onclick=self.link.callback(|_| Msg::ReverseList)>
+                        { "Reverse list" }
+                    </button>
+                    <button onclick=self.link.callback(|_| Msg::SortById)>
+                        { "Sort by id" }
+                    </button>
+                    <button onclick=self.link.callback(|_| Msg::SortByName)>
+                        { "Sort by name" }
+                    </button>
+                    <button onclick=self.link.callback(|_| Msg::SortByAge)>
+                        { "Sort by age" }
+                    </button>
+                    <button onclick=self.link.callback(|_| Msg::SortByAddress)>
+                        { "Sort by address" }
+                    </button>
+                    <button onclick=self.link.callback(|_| Msg::ToggleKeyed)>
+                        { if self.keyed { "Disable keys" } else { "Enable keys" } }
+                    </button>
+                </div>
+                <div class="ratio">
+                    <label for="ratio">{ "Person type ratio (0=only tags <= ratio <= 1=only components): " }</label>
+                    <input
+                        class="input" type="text" id="ratio"
+                        value=self.build_component_ratio
+                        oninput=self.link.callback(|e: InputData| Msg::ChangeRatio(e.value))
+                        />
+                </div>
+                <p>{ "Number of persons: " }{ self.persons.len() }</p>
+                <p>{ "Ids: " }{ ids }</p>
+                <hr />
+                <div class="persons">
+                    { for self.persons.iter().map(|p| match p {
+                        PersonType::Inline(info) if self.keyed => {
+                            html! {
+                                <div class="basic-person" key=info.id.to_string() id=info.id.to_string()>
+                                    { info.render() }
+                                </div>
+                            }
+                        },
+                        PersonType::Inline(info) => {
+                            html! {
+                                <div class="basic-person" id=info.id.to_string()>
+                                    { info.render() }
+                                </div>
+                            }
+                        },
+                        PersonType::Component(info) if self.keyed => html! { <PersonComponent info=info key=info.id.to_string() /> },
+                        PersonType::Component(info) => html! { <PersonComponent info=info /> },
+                    })}
+                </div>
+            </>
+        }
+    }
+}
+
+impl Component for PersonComponent {
+    type Message = ();
+    type Properties = PersonProps;
+
+    fn create(props: Self::Properties, _link: ComponentLink<Self>) -> Self {
+        PersonComponent { info: props.info }
+    }
+
+    fn update(&mut self, _msg: Self::Message) -> ShouldRender {
+        false
+    }
+
+    fn change(&mut self, props: Self::Properties) -> ShouldRender {
+        self.info.neq_assign(props.info)
+    }
+
+    fn view(&self) -> Html {
+        html! {
+            <div class="component-person" id=self.info.id.to_string()>
+                { self.info.render() }
+            </div>
+        }
+    }
+}
+
+impl PersonType {
+    fn info(&self) -> &PersonInfo {
+        match self {
+            PersonType::Inline(info) => info,
+            PersonType::Component(info) => info,
+        }
+    }
+
+    fn new_random(id: usize, ratio: f64) -> Self {
+        let info = PersonInfo::new_random(id);
+        if (rand::thread_rng().gen::<f64>() % 1.0) > ratio {
+            PersonType::Inline(info)
+        } else {
+            PersonType::Component(info)
+        }
+    }
+}
+
+impl PersonInfo {
+    fn new_random(id: usize) -> Self {
+        PersonInfo {
+            id,
+            name: Rc::new(PersonInfo::gen_name()),
+            age: PersonInfo::gen_age(),
+            address: Rc::new(PersonInfo::gen_address()),
+        }
+    }
+
+    fn render(&self) -> Html {
+        html! {
+            <div class="person">
+                <h1>{ &self.id }{ " - " }{ &self.name }</h1>
+                <p>{ "Age: " }{ &self.age }</p>
+                <p>{ "Address: " }{ &self.address }</p>
+            </div>
+        }
+    }
+
+    fn gen_number(min: usize, max: usize) -> usize {
+        let len: usize = rand::thread_rng().gen();
+        len % (max - min) + min
+    }
+
+    fn gen_string(len: usize) -> String {
+        let mut rng = rand::thread_rng();
+        (0..len)
+            .map(|_| rng.sample(rand::distributions::Alphanumeric))
+            .collect()
+    }
+
+    fn gen_words(n_words: usize, min_len: usize, max_len: usize) -> Vec<String> {
+        (0..n_words)
+            .map(|_| PersonInfo::gen_string(PersonInfo::gen_number(min_len, max_len)))
+            .collect()
+    }
+
+    fn gen_name() -> String {
+        PersonInfo::gen_words(2, 4, 15).join(" ")
+    }
+
+    fn gen_age() -> usize {
+        PersonInfo::gen_number(7, 77)
+    }
+
+    fn gen_address() -> String {
+        let n_words = PersonInfo::gen_number(3, 6);
+        PersonInfo::gen_words(n_words, 5, 12).join(" ")
+    }
+}

examples/keyed_list/src/main.rs

@@ -0,0 +1,4 @@
+fn main() {
+    wasm_logger::init(wasm_logger::Config::new(log::Level::Trace));
+    yew::start_app::<keyed_list::Model>();
+}

examples/keyed_list/static/index.html

@@ -0,0 +1,11 @@
+<!doctype html>
+<html lang="en">
+    <head>
+        <meta charset="utf-8">
+        <title>Keyed list example</title>
+        <link rel="stylesheet" href="styles.css">
+    </head>
+    <body>
+        <script src="/keyed_list.js"></script>
+    </body>
+</html>

examples/keyed_list/static/styles.css

@@ -0,0 +1,7 @@
+.component-person {
+    color: blue;
+}
+
+.basic-person {
+    color: red;
+}

yew-macro/src/html_tree/html_component.rs

@@ -150,7 +150,7 @@ impl ToTokens for HtmlComponent {
         };
 
         let key = if let Some(key) = &props.key {
-            quote_spanned! { key.span()=> Some(#key) }
+            quote_spanned! { key.span()=> Some(::yew::virtual_dom::Key::from(#key)) }
         } else {
             quote! {None}
         };

yew-macro/src/html_tree/html_list.rs

@@ -69,7 +69,7 @@ impl ToTokens for HtmlList {
     fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) {
         let children = &self.children;
         let key = if let Some(key) = &self.key {
-            quote_spanned! {key.span()=> Some(#key)}
+            quote_spanned! {key.span()=> Some(::yew::virtual_dom::Key::from(#key))}
         } else {
             quote! {None}
         };

yew-macro/src/html_tree/html_tag/mod.rs

@@ -185,7 +185,7 @@ impl ToTokens for HtmlTag {
         });
         let set_key = key.iter().map(|key| {
             quote! {
-                #vtag.key = Some(#key);
+                #vtag.key = Some(::yew::virtual_dom::Key::from(#key));
             }
         });
         let listeners = listeners.iter().map(|listener| {

yew-stdweb/Cargo.toml

@@ -25,6 +25,7 @@ bincode = { version = "~1.2.1", optional = true }
 cfg-if = "0.1"
 cfg-match = "0.2"
 console_error_panic_hook = { version = "0.1", optional = true }
+fixedbitset = "0.3.0"
 futures = { version = "0.3", optional = true }
 gloo = { version = "0.2.1", optional = true }
 http = "0.2"

yew/src/virtual_dom/key.rs

@@ -0,0 +1,91 @@
+//! This module contains the implementation yew's virtual nodes' keys.
+
+use std::rc::Rc;
+
+/// Represents the (optional) key of Yew's virtual nodes.
+///
+/// Keys are cheap to clone.
+// TODO (#1263): Explain when keys are useful and add an example.
+#[derive(Clone, Ord, PartialOrd, Eq, PartialEq, Hash)]
+pub struct Key {
+    key: Rc<String>,
+}
+
+impl core::fmt::Display for Key {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        write!(f, "{}", &self.key)
+    }
+}
+
+impl core::fmt::Debug for Key {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        write!(f, "{}", &self.key)
+    }
+}
+
+impl From<Rc<String>> for Key {
+    fn from(key: Rc<String>) -> Self {
+        Key { key }
+    }
+}
+
+macro_rules! key_impl_from_to_string {
+    ($type:ty) => {
+        impl From<$type> for Key {
+            fn from(key: $type) -> Self {
+                Key {
+                    key: Rc::new(key.to_string()),
+                }
+            }
+        }
+    };
+}
+key_impl_from_to_string!(&'static str);
+key_impl_from_to_string!(String);
+key_impl_from_to_string!(char);
+key_impl_from_to_string!(usize);
+key_impl_from_to_string!(u8);
+key_impl_from_to_string!(u16);
+key_impl_from_to_string!(u32);
+key_impl_from_to_string!(u64);
+key_impl_from_to_string!(u128);
+key_impl_from_to_string!(i8);
+key_impl_from_to_string!(i16);
+key_impl_from_to_string!(i32);
+key_impl_from_to_string!(i64);
+key_impl_from_to_string!(i128);
+
+#[cfg(test)]
+mod test {
+    use crate::html;
+    use std::rc::Rc;
+
+    #[cfg(feature = "wasm_test")]
+    use wasm_bindgen_test::{wasm_bindgen_test as test, wasm_bindgen_test_configure};
+
+    #[cfg(feature = "wasm_test")]
+    wasm_bindgen_test_configure!(run_in_browser);
+
+    #[test]
+    fn all_key_conversions() {
+        let rc_key = Rc::new("rc".to_string());
+        html! {
+            <key="string literal">
+                <img key="String".to_string() />
+                <p key=rc_key></p>
+                <key='a'>
+                    <p key=11_usize></p>
+                    <p key=12_u8></p>
+                    <p key=13_u16></p>
+                    <p key=14_u32></p>
+                    <p key=15_u64></p>
+                    <p key=15_u128></p>
+                    <p key=22_i8></p>
+                    <p key=23_i16></p>
+                    <p key=24_i32></p>
+                    <p key=25_i128></p>
+                </>
+            </>
+        };
+    }
+}

yew/src/virtual_dom/mod.rs

@@ -1,5 +1,7 @@
 //! This module contains Yew's implementation of a reactive virtual DOM.
 
+#[doc(hidden)]
+pub mod key;
 #[doc(hidden)]
 pub mod vcomp;
 #[doc(hidden)]
@@ -27,6 +29,8 @@ cfg_if! {
     }
 }
 
+#[doc(inline)]
+pub use self::key::Key;
 #[doc(inline)]
 pub use self::vcomp::{VChild, VComp};
 #[doc(inline)]
@@ -200,6 +204,8 @@ pub(crate) trait VDiff {
     /// - `ancestor`: the node that this node will be replacing in the DOM. This
     ///   method will _always_ remove the `ancestor` from the `parent`.
     ///
+    /// Returns a reference to the newly inserted element.
+    ///
     /// ### Internal Behavior Notice:
     ///
     /// Note that these modify the DOM by modifying the reference that _already_
@@ -319,6 +325,7 @@ mod layout_tests {
     }
 
     pub(crate) struct TestLayout<'a> {
+        pub(crate) name: &'a str,
         pub(crate) node: VNode,
         pub(crate) expected: &'a str,
     }
@@ -337,14 +344,18 @@ mod layout_tests {
         for layout in layouts.iter() {
             // Apply the layout
             let mut node = layout.node.clone();
+            wasm_bindgen_test::console_log!("Independently apply layout '{}'", layout.name);
             node.apply(&parent_scope, &parent_element, next_sibling.clone(), None);
             assert_eq!(
                 parent_element.inner_html(),
-                format!("{}END", layout.expected)
+                format!("{}END", layout.expected),
+                "Independent apply failed for layout '{}'",
+                layout.name,
             );
 
             // Diff with no changes
             let mut node_clone = layout.node.clone();
+            wasm_bindgen_test::console_log!("Independently reapply layout '{}'", layout.name);
             node_clone.apply(
                 &parent_scope,
                 &parent_element,
@@ -353,7 +364,9 @@ mod layout_tests {
             );
             assert_eq!(
                 parent_element.inner_html(),
-                format!("{}END", layout.expected)
+                format!("{}END", layout.expected),
+                "Independent reapply failed for layout '{}'",
+                layout.name,
             );
 
             // Detach
@@ -363,13 +376,19 @@ mod layout_tests {
                 next_sibling.clone(),
                 Some(node_clone),
             );
-            assert_eq!(parent_element.inner_html(), "END");
+            assert_eq!(
+                parent_element.inner_html(),
+                "END",
+                "Independent detach failed for layout '{}'",
+                layout.name,
+            );
         }
 
         // Sequentially apply each layout
         let mut ancestor: Option<VNode> = None;
         for layout in layouts.iter() {
             let mut next_node = layout.node.clone();
+            wasm_bindgen_test::console_log!("Sequentially apply layout '{}'", layout.name);
             next_node.apply(
                 &parent_scope,
                 &parent_element,
@@ -378,7 +397,9 @@ mod layout_tests {
             );
             assert_eq!(
                 parent_element.inner_html(),
-                format!("{}END", layout.expected)
+                format!("{}END", layout.expected),
+                "Sequential apply failed for layout '{}'",
+                layout.name,
             );
             ancestor = Some(next_node);
         }
@@ -386,6 +407,7 @@ mod layout_tests {
         // Sequentially detach each layout
         for layout in layouts.into_iter().rev() {
             let mut next_node = layout.node.clone();
+            wasm_bindgen_test::console_log!("Sequentially detach layout '{}'", layout.name);
             next_node.apply(
                 &parent_scope,
                 &parent_element,
@@ -394,7 +416,9 @@ mod layout_tests {
             );
             assert_eq!(
                 parent_element.inner_html(),
-                format!("{}END", layout.expected)
+                format!("{}END", layout.expected),
+                "Sequential detach failed for layout '{}'",
+                layout.name,
             );
             ancestor = Some(next_node);
         }
@@ -403,6 +427,10 @@ mod layout_tests {
         empty_node
             .clone()
             .apply(&parent_scope, &parent_element, next_sibling, ancestor);
-        assert_eq!(parent_element.inner_html(), "END");
+        assert_eq!(
+            parent_element.inner_html(),
+            "END",
+            "Failed to detach last layout"
+        );
     }
 }

yew/src/virtual_dom/vcomp.rs

@@ -1,6 +1,6 @@
 //! This module contains the implementation of a virtual component (`VComp`).
 
-use super::{Transformer, VDiff, VNode};
+use super::{Key, Transformer, VDiff, VNode};
 use crate::html::{AnyScope, Component, ComponentUpdate, NodeRef, Scope, Scoped};
 use crate::utils::document;
 use cfg_if::cfg_if;
@@ -22,7 +22,7 @@ pub struct VComp {
     scope: Option<Box<dyn Scoped>>,
     props: Option<Box<dyn Mountable>>,
     pub(crate) node_ref: NodeRef,
-    pub(crate) key: Option<String>,
+    pub(crate) key: Option<Key>,
 }
 
 impl Clone for VComp {
@@ -47,7 +47,7 @@ pub struct VChild<COMP: Component> {
     pub props: COMP::Properties,
     /// Reference to the mounted node
     node_ref: NodeRef,
-    key: Option<String>,
+    key: Option<Key>,
 }
 
 impl<COMP: Component> Clone for VChild<COMP> {
@@ -74,7 +74,7 @@ where
     COMP: Component,
 {
     /// Creates a child component that can be accessed and modified by its parent.
-    pub fn new(props: COMP::Properties, node_ref: NodeRef, key: Option<String>) -> Self {
+    pub fn new(props: COMP::Properties, node_ref: NodeRef, key: Option<Key>) -> Self {
         Self {
             props,
             node_ref,
@@ -94,7 +94,7 @@ where
 
 impl VComp {
     /// Creates a new `VComp` instance.
-    pub fn new<COMP>(props: COMP::Properties, node_ref: NodeRef, key: Option<String>) -> Self
+    pub fn new<COMP>(props: COMP::Properties, node_ref: NodeRef, key: Option<Key>) -> Self
     where
         COMP: Component,
     {
@@ -185,7 +185,7 @@ impl VDiff for VComp {
         if let Some(mut ancestor) = ancestor {
             if let VNode::VComp(ref mut vcomp) = &mut ancestor {
                 // If the ancestor is the same type, reuse it and update its properties
-                if self.type_id == vcomp.type_id {
+                if self.type_id == vcomp.type_id && self.key == vcomp.key {
                     self.node_ref.link(vcomp.node_ref.clone());
                     let scope = vcomp.scope.take().expect("VComp is not mounted");
                     mountable.reuse(scope.borrow(), next_sibling);
@@ -349,7 +349,7 @@ mod tests {
 
     #[test]
     fn set_component_key() {
-        let test_key = "test".to_string();
+        let test_key: Key = "test".to_string().into();
         let check_key = |vnode: VNode| {
             assert_eq!(vnode.key().as_ref(), Some(&test_key));
         };
@@ -578,6 +578,7 @@ mod layout_tests {
     #[test]
     fn diff() {
         let layout1 = TestLayout {
+            name: "1",
             node: html! {
                 <Comp<A>>
                     <Comp<B>></Comp<B>>
@@ -588,6 +589,7 @@ mod layout_tests {
         };
 
         let layout2 = TestLayout {
+            name: "2",
             node: html! {
                 <Comp<A>>
                     {"A"}
@@ -597,6 +599,7 @@ mod layout_tests {
         };
 
         let layout3 = TestLayout {
+            name: "3",
             node: html! {
                 <Comp<B>>
                     <Comp<A>></Comp<A>>
@@ -607,6 +610,7 @@ mod layout_tests {
         };
 
         let layout4 = TestLayout {
+            name: "4",
             node: html! {
                 <Comp<B>>
                     <Comp<A>>{"A"}</Comp<A>>
@@ -617,6 +621,7 @@ mod layout_tests {
         };
 
         let layout5 = TestLayout {
+            name: "5",
             node: html! {
                 <Comp<B>>
                     <>
@@ -631,6 +636,7 @@ mod layout_tests {
         };
 
         let layout6 = TestLayout {
+            name: "6",
             node: html! {
                 <Comp<B>>
                     <>
@@ -646,6 +652,7 @@ mod layout_tests {
         };
 
         let layout7 = TestLayout {
+            name: "7",
             node: html! {
                 <Comp<B>>
                     <>
@@ -663,6 +670,7 @@ mod layout_tests {
         };
 
         let layout8 = TestLayout {
+            name: "8",
             node: html! {
                 <Comp<B>>
                     <>
@@ -682,6 +690,7 @@ mod layout_tests {
         };
 
         let layout9 = TestLayout {
+            name: "9",
             node: html! {
                 <Comp<B>>
                     <>
@@ -701,6 +710,7 @@ mod layout_tests {
         };
 
         let layout10 = TestLayout {
+            name: "10",
             node: html! {
                 <Comp<B>>
                     <>
@@ -720,6 +730,7 @@ mod layout_tests {
         };
 
         let layout11 = TestLayout {
+            name: "11",
             node: html! {
                 <Comp<B>>
                     <>
@@ -739,6 +750,7 @@ mod layout_tests {
         };
 
         let layout12 = TestLayout {
+            name: "12",
             node: html! {
                 <Comp<B>>
                     <>

yew/src/virtual_dom/vnode.rs

@@ -1,6 +1,6 @@
 //! This module contains the implementation of abstract virtual node.
 
-use super::{VChild, VComp, VDiff, VList, VTag, VText};
+use super::{Key, VChild, VComp, VDiff, VList, VTag, VText};
 use crate::html::{AnyScope, Component, NodeRef, Renderable};
 use cfg_if::cfg_if;
 use cfg_match::cfg_match;
@@ -33,6 +33,16 @@ pub enum VNode {
 }
 
 impl VNode {
+    pub fn key(&self) -> Option<Key> {
+        match self {
+            VNode::VComp(vcomp) => vcomp.key.clone(),
+            VNode::VList(vlist) => vlist.key.clone(),
+            VNode::VRef(_) => None,
+            VNode::VTag(vtag) => vtag.key.clone(),
+            VNode::VText(_) => None,
+        }
+    }
+
     /// Returns the first DOM node that is used to designate the position of the virtual DOM node.
     pub(crate) fn first_node(&self) -> Node {
         match self {
@@ -59,14 +69,21 @@ impl VNode {
         }
     }
 
-    pub fn key(&self) -> &Option<String> {
+    pub(crate) fn move_before(&self, parent: &Element, next_sibling: Option<Node>) {
         match self {
-            VNode::VTag(vtag) => &vtag.key,
-            VNode::VText(_) => &None,
-            VNode::VComp(vcomp) => &vcomp.key,
-            VNode::VList(vlist) => &vlist.key,
-            VNode::VRef(_) => &None,
-        }
+            VNode::VList(vlist) => {
+                for node in vlist.children.iter() {
+                    node.move_before(parent, next_sibling.clone());
+                }
+            }
+            VNode::VComp(vcomp) => {
+                vcomp
+                    .root_vnode()
+                    .expect("VComp has no root vnode")
+                    .move_before(parent, next_sibling);
+            }
+            _ => super::insert_node(&self.first_node(), parent, next_sibling),
+        };
     }
 }
 
@@ -200,7 +217,7 @@ impl PartialEq for VNode {
             (VNode::VText(a), VNode::VText(b)) => a == b,
             (VNode::VList(a), VNode::VList(b)) => a == b,
             (VNode::VRef(a), VNode::VRef(b)) => a == b,
-            // Need to improve PartialEq for VComp before enabling
+            // TODO: Need to improve PartialEq for VComp before enabling.
             (VNode::VComp(_), VNode::VComp(_)) => false,
             _ => false,
         }
@@ -225,11 +242,13 @@ mod layout_tests {
         let vref_node_2 = VNode::VRef(document.create_element("b").unwrap().into());
 
         let layout1 = TestLayout {
+            name: "1",
             node: vref_node_1.into(),
             expected: "<i></i>",
         };
 
         let layout2 = TestLayout {
+            name: "2",
             node: vref_node_2.into(),
             expected: "<b></b>",
         };

yew/src/virtual_dom/vtag.rs

@@ -1,6 +1,6 @@
 //! This module contains the implementation of a virtual element node `VTag`.
 
-use super::{Attributes, Listener, Listeners, Patch, Transformer, VDiff, VList, VNode};
+use super::{Attributes, Key, Listener, Listeners, Patch, Transformer, VDiff, VList, VNode};
 use crate::html::{AnyScope, NodeRef};
 use crate::utils::document;
 use cfg_if::cfg_if;
@@ -62,7 +62,7 @@ pub struct VTag {
     tag: Cow<'static, str>,
     /// Type of element.
     element_type: ElementType,
-    /// A reference to the `Element`.
+    /// A reference to the DOM `Element`.
     pub reference: Option<Element>,
     /// List of attached listeners.
     pub listeners: Listeners,
@@ -88,7 +88,7 @@ pub struct VTag {
     /// Keeps handler for attached listeners to have an opportunity to drop them later.
     captured: Vec<EventListener>,
 
-    pub key: Option<String>,
+    pub key: Option<Key>,
 }
 
 impl Clone for VTag {
@@ -273,7 +273,7 @@ impl VTag {
         to_add_or_replace.chain(to_remove)
     }
 
-    /// Similar to `diff_attributers` except there is only a single `kind`.
+    /// Similar to `diff_attributes` except there is only a single `kind`.
     fn diff_kind<'a>(&'a self, ancestor: &'a Option<Box<Self>>) -> Option<Patch<&'a str, ()>> {
         match (
             self.kind.as_ref(),
@@ -419,9 +419,10 @@ impl VTag {
                 .namespace_uri()
                 .map_or(false, |ns| ns == SVG_NAMESPACE)
         {
-            let namespace = SVG_NAMESPACE;
-            #[cfg(feature = "web_sys")]
-            let namespace = Some(namespace);
+            let namespace = cfg_match! {
+                feature = "std_web" => SVG_NAMESPACE,
+                feature = "web_sys" => Some(SVG_NAMESPACE),
+            };
             document()
                 .create_element_ns(namespace, &self.tag)
                 .expect("can't create namespaced element for vtag")
@@ -461,7 +462,7 @@ impl VDiff for VTag {
             match ancestor {
                 // If the ancestor is a tag of the same type, don't recreate, keep the
                 // old tag and update its attributes and children.
-                VNode::VTag(vtag) if self.tag == vtag.tag => Some(vtag),
+                VNode::VTag(vtag) if self.tag == vtag.tag && self.key == vtag.key => Some(vtag),
                 _ => {
                     let element = self.create_element(parent);
                     super::insert_node(&element, parent, Some(ancestor.first_node()));
@@ -1308,6 +1309,7 @@ mod layout_tests {
     #[test]
     fn diff() {
         let layout1 = TestLayout {
+            name: "1",
             node: html! {
                 <ul>
                     <li>
@@ -1322,6 +1324,7 @@ mod layout_tests {
         };
 
         let layout2 = TestLayout {
+            name: "2",
             node: html! {
                 <ul>
                     <li>
@@ -1339,6 +1342,7 @@ mod layout_tests {
         };
 
         let layout3 = TestLayout {
+            name: "3",
             node: html! {
                 <ul>
                     <li>
@@ -1359,6 +1363,7 @@ mod layout_tests {
         };
 
         let layout4 = TestLayout {
+            name: "4",
             node: html! {
                 <ul>
                     <li>

yew/src/virtual_dom/vtext.rs

@@ -47,7 +47,7 @@ impl VDiff for VText {
         }
     }
 
-    /// Renders virtual node over existing `TextNode`, but only if value of text had changed.
+    /// Renders virtual node over existing `TextNode`, but only if value of text has changed.
     fn apply(
         &mut self,
         _parent_scope: &AnyScope,
@@ -120,16 +120,19 @@ mod layout_tests {
     #[test]
     fn diff() {
         let layout1 = TestLayout {
+            name: "1",
             node: html! { "a" },
             expected: "a",
         };
 
         let layout2 = TestLayout {
+            name: "2",
             node: html! { "b" },
             expected: "b",
         };
 
         let layout3 = TestLayout {
+            name: "3",
             node: html! {
                 <>
                     {"a"}
@@ -140,6 +143,7 @@ mod layout_tests {
         };
 
         let layout4 = TestLayout {
+            name: "4",
             node: html! {
                 <>
                     {"b"}