unsafe_inner_html does not work with DomUpdater

[FinnStokes]

When running client-side, the unsafe_inner_html attribute does not update the inner html when passed to DomUpdater. For example, the second and third assertions fail in:

#![feature(proc_macro_hygiene)]

extern crate wasm_bindgen_test;

use wasm_bindgen::JsCast;
use wasm_bindgen_test::*;
use web_sys::Element;

use virtual_dom_rs::prelude::*;

wasm_bindgen_test_configure!(run_in_browser);

#[wasm_bindgen_test]
fn test_inner() {
    let div = html!{
        <div unsafe_inner_html="<span>hi</span>"></div>
    };
    let mut updater = DomUpdater::new(div);
    let node: Element = updater.root_node().unchecked_into();
    assert_eq!(node.inner_html(), "<span>hi</span>");

    let div = html!{
        <div><span>hello</span></div>
    };
    updater.update(div);
    let node: Element = updater.root_node().unchecked_into();
    assert_eq!(node.inner_html(), "<span>hello</span>");

    let div = html!{
        <div unsafe_inner_html="<span>goodbye</span>"></div>
    };
    updater.update(div);
    let node: Element = updater.root_node().unchecked_into();
    assert_eq!(node.inner_html(), "<span>goodbye</span>");
}

[FinnStokes]

I'm not sure if there is a good way to fix this, as the inner html is just sort of shoved in there without really interacting with the diffing. For a similar reason it also doesn't work with to_string which isn't ideal from an isomorphism standpoint, but at least can be worked around somewhat easily.

I wanted to use this to render user-provided markdown. Since unsafe_inner_html didn't work for me, I ended up writing methods to transform the parsed and sanitised markdown into IterableNodes that I can then use in a braced block in html!. My current workflow is

markdown --pulldown_cmark--> html --ammonia--> sanitised html5ever tree --percy--> dom updates

This seems to work but I haven't done much testing or benchmarking yet, so I don't know how well.

To convert the html5ever tree, I wrote a trait:

use html5ever::rcdom::{Handle, NodeData};
use virtual_node::{IterableNodes, VirtualNode};

pub trait DomCast {
    fn to_vdom_nodes(&self) -> IterableNodes;
    fn to_vdom_node(&self) -> VirtualNode;
}

impl DomCast for Handle {
    fn to_vdom_nodes(&self) -> IterableNodes {
        self.children.borrow()
                    .iter()
                    .map(DomCast::to_vdom_node)
                    .collect::<Vec<VirtualNode>>()
                    .into()
    }

    fn to_vdom_node(&self) -> VirtualNode {
        let children = self.children.borrow();
        match &self.data {
            NodeData::Text{contents} => VirtualNode::text(contents.borrow().as_ref()).into(),
            NodeData::Element{name, attrs, ..} => {
                let mut node = VirtualNode::element(&*name.local);
                let velement = node.as_velement_mut().expect("Not an element");
                for a in attrs.borrow().iter() {
                    velement.attrs.insert(String::from(&*a.name.local),
                                          String::from(a.value.as_ref()));
                }
                velement.children.extend(
                    children.iter().map(DomCast::to_vdom_node)
                );
                node
            },
            _ => VirtualNode::text(""),
        }
    }
}

This would be a bit more ergonomic as a From implementation for IterableNodes in percy itself, but I'm not sure how good a fit it would be. Being able to drop html5ever trees straight into html! macros would be convenient, but this would require an extra dependency (presumably gated behind a feature). I'm also not sure how stable the html5ever::rcdom api is. I know I have to use the ammonia_unstable flag to even be able to access the html5ever tree from ammonia, which doesn't directly affect this trait, but suggests that the ammonia devs don't trust the stability of the rcdom api (see https://docs.rs/ammonia/3.0.0/src/ammonia/lib.rs.html#1945-2005).

[chinedufn]

Thanks for the terrific breakdown!

~Hmm - could you impl Into<VirtualNode> for &dyn DomCast, or am I missing something?~ nevermind that isn't possible

---

Also that unsafe_inner_html diff/patch story is pretty poor - thanks for pointing that out. Admittedly I added in that feature to satisfy my one use case and it isn't very battle tested.

In terms of ways to fix it - I'm sure the diffing/patching algorithm can be tweaked to be more aware of unsafe_inner_html. Hmmm.

[chinedufn]

Properly patching inner HTML is now tested and works.

https://github.com/chinedufn/percy/blob/76867ce6f62cc17f2705048b7bff121e5e9bbe11/crates/percy-dom/tests/dangerous_inner_html.rs#L35-L61

---

Documentation:

https://github.com/chinedufn/percy/tree/09309ef1b7cd24b4c9ed0d4ccdb3974cee8b0028/book/src/html-macro/setting-inner-html

---

With regards to the html5ever support -> there are no plans for this at the moment as we work to simplify the API.

But if there are any ways to make your use case easier definitely feel free to open an issue.

Also, if you don't think that this issue should be closed out feel free to drop a comment.

Cheers!