Skip to content

Commit

Permalink
Fix replacing node with text
Browse files Browse the repository at this point in the history
  • Loading branch information
chinedufn committed Jan 31, 2019
1 parent cb41f08 commit d9c1cd7
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 40 deletions.
2 changes: 1 addition & 1 deletion crates/html-macro/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@ proc-macro = true
[dependencies]
proc-macro2 = "0.4"
quote = "0.6.11"
syn = { version = "0.15", features = ["full", "extra-traits"] }
syn = { version = "0.15", features = ["full", "extra-traits"] }
15 changes: 0 additions & 15 deletions crates/virtual-dom-rs/src/diff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,21 +264,6 @@ mod tests {
.test();
}

// https://github.com/chinedufn/percy/issues/62
#[test]
fn issue_62() {
DiffTestCase {
old: html! { <span> <br /> </span> },
new: html! { <span> a <br /> </span> },
expected: vec![
Patch::Replace(1, &VirtualNode::text("a")),
Patch::AppendChildren(0, vec![&VirtualNode::new("br")]),
],
description: "Replace text node",
}
.test();
}

// // TODO: Key support
// #[test]
// fn reorder_chldren() {
Expand Down
9 changes: 7 additions & 2 deletions crates/virtual-dom-rs/src/patch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,13 @@ fn apply_element_patch(node: &Element, patch: &Patch) {
}
}
Patch::Replace(_node_idx, new_node) => {
node.replace_with_with_node_1(&new_node.create_element())
.expect("Replaced element");
if new_node.is_text_node() {
node.replace_with_with_node_1(&new_node.create_text_node())
.expect("Replaced with text node");
} else {
node.replace_with_with_node_1(&new_node.create_element())
.expect("Replaced with element");
}
}
Patch::TruncateChildren(_node_idx, num_children_remaining) => {
let children = node.child_nodes();
Expand Down
21 changes: 10 additions & 11 deletions crates/virtual-dom-rs/tests/diff_patch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ fn text_node_siblings() {
// @see virtual_node/mod.rs -> create_element() for more information
let override_expected = Some(
r#"<div id="after"><span>The button has been clicked: <!--ptns-->world</span></div>"#
.to_string(),
);

let old1 = VirtualNode::text("The button has been clicked: ");
Expand Down Expand Up @@ -155,13 +154,13 @@ fn replace_with_children() {
}

// https://github.com/chinedufn/percy/issues/62
//#[wasm_bindgen_test]
//fn issue_62() {
// DiffPatchTest {
// desc: "Fix issue #62",
// old: html! { <span> <br> </span> },
// new: html! { <span> a <br> </span> },
// override_expected: None,
// }
// .test();
//}
#[wasm_bindgen_test]
fn replace_element_with_text_node() {
DiffPatchTest {
desc: "#62: Replace element with text node",
old: html! { <span> <br> </span> },
new: html! { <span> a </span> },
override_expected: None
}
.test();
}
23 changes: 14 additions & 9 deletions crates/virtual-dom-rs/tests/diff_patch_test_case/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ use std::collections::hash_map::DefaultHasher;
use std::hash::{Hash, Hasher};

/// A test case that both diffing and patching are working in a real browser
pub struct DiffPatchTest {
pub struct DiffPatchTest<'a> {
pub desc: &'static str,
pub old: VirtualNode,
pub new: VirtualNode,
pub override_expected: Option<String>,
pub override_expected: Option<&'a str>,
}

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

Expand Down Expand Up @@ -49,14 +49,19 @@ impl DiffPatchTest {

virtual_dom_rs::patch(root_node, &patches);

let expected_new_root_node = match self.override_expected {
Some(ref expected) => expected.clone(),
None => self.new.to_string(),
};
let expected_new_root_node = self.new.to_string();
let mut expected_new_root_node = expected_new_root_node.as_str();

if let Some(ref expected) = self.override_expected {
expected_new_root_node = expected;
}

web_sys::console::log_1(&format!("NEW NODE {:#?}", patched_element.outer_html()).into());
web_sys::console::log_1(&format!("Outter HTML {:#?}", expected_new_root_node).into());

assert_eq!(
patched_element.outer_html(),
expected_new_root_node,
&patched_element.outer_html(),
&expected_new_root_node,
"{}",
self.desc
);
Expand Down
38 changes: 36 additions & 2 deletions crates/virtual-node/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
// Around in order to get rid of dependencies that we don't need in non wasm32 targets

pub use std::cell::RefCell;
use std::collections::HashMap;
use std::collections::{HashSet,HashMap};
use std::fmt;
pub use std::rc::Rc;

Expand All @@ -28,12 +28,28 @@ use lazy_static::lazy_static;
use std::ops::Deref;
use std::sync::Mutex;


// Used to uniquely identify elements that contain closures so that the DomUpdater can
// look them up by their unique id.
// When the DomUpdater sees that the element no longer exists it will drop all of it's
// Rc'd Closures for those events.
lazy_static! {
static ref ELEM_UNIQUE_ID: Mutex<u32> = Mutex::new(0);

static ref SELF_CLOSING_TAGS: HashSet<&'static str> = {
let mut set = HashSet::new();

let mut self_closing = vec![
"area", "base", "br", "col", "hr", "img", "input", "link", "meta", "param", "command",
"keygen", "source",
];

for tag in self_closing {
set.insert(tag);
}

set
};
}

/// When building your views you'll typically use the `html!` macro to generate
Expand Down Expand Up @@ -233,6 +249,11 @@ impl VirtualNode {
pub fn is_text_node(&self) -> bool {
self.text.is_some()
}

/// Whether or not this is a self closing tag such as <br> or <img />
pub fn is_self_closing(&self) -> bool {
SELF_CLOSING_TAGS.contains(self.tag.as_str())
}
}

impl From<&str> for VirtualNode {
Expand Down Expand Up @@ -288,7 +309,12 @@ impl fmt::Display for VirtualNode {
for child in self.children.as_ref().unwrap().iter() {
write!(f, "{}", child.to_string())?;
}
write!(f, "</{}>", self.tag)

if !self.is_self_closing() {
write!(f, "</{}>", self.tag)?;
}

Ok(())
}
}
}
Expand Down Expand Up @@ -320,6 +346,14 @@ impl fmt::Debug for Events {
mod tests {
use super::*;

#[test]
fn self_closing_tag_to_string() {
let node = VirtualNode::new("br");

// No </br> since self closing tag
assert_eq!(&node.to_string(), "<br>");
}

// TODO: Use html_macro as dev dependency and uncomment
// #[test]
// fn to_string() {
Expand Down

0 comments on commit d9c1cd7

Please sign in to comment.