Structure of VirtualNode about percy HOT 7 CLOSED

For example for patch:

pub fn patch<N: Into<Node>>(root_node: N, patches: &Vec<Patch>) {
    let root_node = root_node.into();
    ...

This way you can patch a Node, an Element or any other subtype of Node.

from percy.

Hi. I started a refactoring of the VirtualNode struct.

Awesome! It's in dire need of some love!

This is both a bit ugly and causes confusion. It also makes it possible to represent illegal states (e.g. a text node with children).

Totally agree!

Is this intended, do we want the root node to always be an element node? Or should I convert the DomUpdater to use a web_sys::Node as root?

So Element is there for no other reason than that it currently makes the tests pass.

This is because the dom updater tests are young and immature and don't test for text root nodes.

So I'm thinking we need another dom updater test in that same file looks something like:

#[wasm_bindgen_test]
fn text_root_node () {
    let document = web_sys::window().unwrap().document().unwrap();

    let vdom = html! { Old text };
    let mut dom_updater = DomUpdater::new(vdom);

    let new_vdom = html! { New text };
    dom_updater.update(new_vdom);

    let wrapper = document.create_element("div").unwrap();

    document
        .body()
        .unwrap()
        .append_child(&wrapper);

    wrapper
        .append_child(&dom_updater.root_node());

    assert_eq!(&wrapper.inner_html(), "New text");
}

and then DomUpdater can use web_sys::Node in order to make that test pass.

Also, are you aware of an enum or trait that covers all web_sys subtypes of Node, like Element or Text?

Not entirely sure what you mean here. What are you looking to do? As in something that would let you use a trait object to accept any of those subtypes? (Sorry if I'm missing something!)

Thanks for diving into this! As always I'm happy to answer any questions / bounce ideas! This already sounds way cleaner!

from percy.

Not entirely sure what you mean here. What are you looking to do? As in something that would let you use a trait object to accept any of those subtypes? (Sorry if I'm missing something!)

It would allow me to implement as_dom_node(&self) on VirtualNode directly, which would then delegate the implementation to the variants. Right now we always have to match on the variants, and then call as_(text|element)_node on there.

Thanks for the test case!

from percy.

Ahhhh gotcha thanks!

Yeah I haven't come across anything for this so matching sounds great for now.

Also heads up just added another failing test case in a different file (currently commented out) to complement the example that I posted above

824a44f

Also also these changes will certainly make the html macro tests fail - so let me know if/where/how I can help there!

from percy.

Yeah I haven't come across anything for this so matching sounds great for now.

I just realized that I can use web_sys::Node and simply dyn_cast the child types into the Node type! Won't work for all purposes, but for many.

from percy.

Ahhh good point - forgot that we do that but it looks like we do do that when patching text nodes

Sweet!

from percy.

Actually it's even simpler. As per https://rustwasm.github.io/rfcs/002-wasm-bindgen-inheritance-casting.html#example-upcasting you can upcast using into().

let text: Text = create_text();
let element: Element = create_element();
let node1: Node = text.into();
let node2: Node = element.into();

This is very nice in combination with an Into<Node> trait bound on functions:

fn do_something_with_node<N: Into<Node>>(node: N) {
    do_something(node.into());
}

from percy.