Fixing diff/patch issues

As our virtual dom implementation ages it will become more and more resilient, but while we're still an experimental library it's possible that the diff/patch algorithm could fail in some scenarios.

If you notice a failure the first step is to open a new issue.

Ideally you include an example start node and end node that isn't working properly.

Let's make up an example here.

# Example things that you'd include in your issue.

start: html! { <div> </div>  }

end: html! { <span> </span> }

Observed error: It somehow ends up as <b></b> in my browser!

If you've opened this issue you've already made a big contribution!

If you'd like to go further, here's how to get to the root of the problem.

Debugging Failed Diff

The easiest place to start is by adding a new diff test and seeing what patches you get.

#![allow(unused)]

fn main() {
use crate::diff::diff;
use crate::patch::Patch;
use virtual_node::VirtualNode;

/// Test that we generate the right Vec<Patch> for some start and end virtual dom.
pub struct DiffTestCase<'a> {
    // ex: html! { <div> </div> }
    pub old: VirtualNode,
    // ex: html! { <strong> </strong> }
    pub new: VirtualNode,
    // ex: vec![Patch::Replace(0, &html! { <strong></strong> })],
    pub expected: Vec<Patch<'a>>,
}

impl<'a> DiffTestCase<'a> {
    pub fn test(&self) {
        // ex: vec![Patch::Replace(0, &html! { <strong></strong> })],
        let patches = diff(&self.old, &self.new);

        assert_eq!(patches, self.expected);
    }
}
}

Diff patch tests get added in diff.rs. Here's an example:

#![allow(unused)]
fn main() {
// diff.rs

#[test]
fn add_children() {
   DiffTestCase {
       old: html! { <div> <b></b> </div> },
       new: html! { <div> <b></b> <new></new> </div> },
       expected: vec![Patch::AppendChildren(0, vec![&html! { <new></new> }])],
       description: "Added a new node to the root node",
   }.test();
}
}

To run your new test case:

# To run just your new diff test
cargo test -p percy-dom --lib my_new_test_name_here

# To run all diff tests
cargo test -p percy-dom --lib diff::tests

If things are failing then you've found the issue!

Please comment back on your original issue with your findings.

If everything is passing, then it must be a patching issue.

Debugging Failed Patch

If the diff checked out, then the issue must be in the patching process.

Patches are tested in crates/percy-dom/tests/diff_patch.rs

A patch test case looks like this:

#![allow(unused)]
fn main() {
//! Kept in its own file to more easily import into the book

use console_error_panic_hook;
use percy_dom::event::VirtualEvents;
use percy_dom::prelude::*;
use wasm_bindgen::JsCast;
use web_sys::{Element, Node};

/// A test case that both diffing and patching are working in a real browser
pub struct DiffPatchTest<'a> {
    /// Description of the test case.
    /// TODO: Delete the description.. not that useful and easy to forget to update after
    ///  copy/pasting another similar test.
    pub desc: &'static str,
    /// The old virtual node.
    pub old: VirtualNode,
    /// The new virtual node.
    pub new: VirtualNode,
    /// By default we generate the expected based on `new.to_string()`. You can
    /// use this field to override the expected HTML after patching.
    pub override_expected: Option<&'a str>,
}

impl<'a> DiffPatchTest<'a> {
    pub fn test(&mut self) {
        console_error_panic_hook::set_once();

        let mut events = VirtualEvents::new();

        // Create a DOM node of the virtual root node
        let (root_node, enode) = self.old.create_dom_node(&mut events);
        events.set_root(enode);

        // Clone since percy_dom::patch takes ownership of the root node.
        let patched_root_node: Node = root_node.clone();

        // Generate patches
        let patches = percy_dom::diff(&self.old, &self.new);

        // Patch our root node. It should now look like `self.new`
        percy_dom::patch(root_node, &self.new, &mut events, &patches).unwrap();

        // Determine the expected outer HTML
        let expected_outer_html = match self.override_expected {
            Some(ref expected) => expected.to_string(),
            None => self.new.to_string(),
        };

        let actual_outer_html = match patched_root_node.node_type() {
            Node::ELEMENT_NODE => patched_root_node.unchecked_into::<Element>().outer_html(),
            Node::TEXT_NODE => patched_root_node.text_content().unwrap_or("".into()),
            _ => panic!("Unhandled node type"),
        };

        assert_eq!(&actual_outer_html, &expected_outer_html, "{}", self.desc);
    }
}
}

#![allow(unused)]
fn main() {
// Example diff patch test case.
// Found in `crates/percy-dom/tests/diff_patch.rs`

use percy_dom::prelude::*;

wasm_bindgen_test_configure!(run_in_browser);

mod diff_patch_test_case;
use self::diff_patch_test_case::DiffPatchTest;

#[wasm_bindgen_test]
fn truncate_children() {
    DiffPatchTest {
        desc: "Truncates extra children",
        old: html! {
         <div>
           <div> <div> <b></b> <em></em> </div> </div>
}

# Run just your new diff patch test
wasm-pack test --chrome --headless crates/percy-dom --test diff_patch -- my_test_name_here

# Run all diff patch tests that contain the word replace
wasm-pack test --chrome --headless crates/percy-dom --test diff_patch -- replace

# Run all diff patch tests
wasm-pack test --chrome --headless crates/percy-dom --test diff_patch

Create your new test case and run it to see if things fail.

If they do, update your original issue with your findings.

Fixing the problem

Look at the documentation for the diff algorithm and the patch algorithm to get a good sense of where and how our diffing and patching is implemented. Fixing the problem will require you to dive into that code.

As you go, if you see opportunities to make the code more understandable, DRY or better commented, seize them!

Look through your errors and try to pinpoint the exact place that the bug is stemming from. If you're stuck, continue to update your issue with your questions and progress and someone will get back to you.