diff --git a/Cargo.lock b/Cargo.lock index e60f01b..e9df6b4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -195,7 +195,7 @@ dependencies = [ [[package]] name = "respo" -version = "0.1.12" +version = "0.1.13" dependencies = [ "cirru_parser", "js-sys", diff --git a/demo_respo/src/inner_text.rs b/demo_respo/src/inner_text.rs new file mode 100644 index 0000000..9414170 --- /dev/null +++ b/demo_respo/src/inner_text.rs @@ -0,0 +1,57 @@ +//! a demo for switching inner-text and children, which might cause a bug in respo + +use std::fmt::Debug; + +use respo::{button, css::RespoStyle, div, span, ui::ui_button, util, DispatchFn, RespoElement, RespoEvent}; +use respo_state_derive::RespoState; +use serde::{Deserialize, Serialize}; + +use respo::states_tree::{RespoState, RespoStatesTree}; + +use super::store::ActionOp; + +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize, RespoState)] +struct InnerTextState { + inner_text: bool, +} + +pub fn comp_inner_text(states: &RespoStatesTree) -> Result, String> { + let cursor = states.path(); + + let state = states.cast_branch::(); + + let on_inc = { + let cursor = cursor.to_owned(); + let state = state.to_owned(); + move |e, dispatch: DispatchFn<_>| -> Result<(), String> { + util::log!("click {:?}", e); + if let RespoEvent::Click { original_event, .. } = e { + original_event.prevent_default(); + } + + dispatch.run(ActionOp::Increment)?; + dispatch.run_state( + &cursor, + InnerTextState { + inner_text: !state.inner_text, + }, + )?; + Ok(()) + } + }; + + Ok( + div().elements([ + div().elements([button() + .class(ui_button()) + .inner_text("Switch inner text") + .style(RespoStyle::default().margin(4.)) + .on_click(on_inc)]), + div().elements([if state.inner_text { + div().inner_text("inner text") + } else { + div().elements([span().inner_text("child 1"), span().inner_text("child 2")]) + }]), + ]), + ) +} diff --git a/demo_respo/src/main.rs b/demo_respo/src/main.rs index 7f3fb87..1afeb31 100644 --- a/demo_respo/src/main.rs +++ b/demo_respo/src/main.rs @@ -1,6 +1,7 @@ extern crate console_error_panic_hook; mod counter; +mod inner_text; mod panel; mod plugins; mod store; @@ -11,7 +12,8 @@ use std::cell::{Ref, RefCell}; use std::panic; use std::rc::Rc; -use respo::RespoAction; +use inner_text::comp_inner_text; +use respo::{space, RespoAction}; use web_sys::Node; use respo::ui::ui_global; @@ -65,9 +67,14 @@ impl RespoApp for App { .style(RespoStyle::default().padding(12.0)) .children([ comp_counter(&states.pick("counter"), store.counted)?.to_node(), + space(None, Some(80)).to_node(), comp_panel(&states.pick("panel"))?, comp_todolist(&states.pick("todolist"), &store.tasks)?.to_node(), + space(None, Some(80)).to_node(), comp_plugins_demo(&states.pick("plugins-demo"))?.to_node(), + space(None, Some(80)).to_node(), + comp_inner_text(&states.pick("inner-text"))?.to_node(), + space(None, Some(80)).to_node(), ]) .to_node(), ) diff --git a/respo/Cargo.toml b/respo/Cargo.toml index aef374b..c517db4 100644 --- a/respo/Cargo.toml +++ b/respo/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "respo" -version = "0.1.12" +version = "0.1.13" edition = "2021" description = "a tiny virtual DOM library migrated from ClojureScript" license = "Apache-2.0" diff --git a/respo/src/app/diff.rs b/respo/src/app/diff.rs index 273af4c..1fa99f2 100644 --- a/respo/src/app/diff.rs +++ b/respo/src/app/diff.rs @@ -1,3 +1,4 @@ +use std::cell::RefCell; use std::collections::{HashMap, HashSet}; use std::fmt::Debug; use std::rc::Rc; @@ -109,7 +110,8 @@ where }); collect_effects_outside_in_as(new_tree, coord, dom_path, RespoEffectType::Mounted, changes)?; } else { - diff_attrs(attrs, old_attrs, coord, dom_path, changes); + let reset_inner = RefCell::new(false); + diff_attrs(attrs, old_attrs, coord, dom_path, changes, &reset_inner); diff_style( &HashMap::from_iter(style.0.to_owned()), &HashMap::from_iter(old_style.0.to_owned()), @@ -119,7 +121,12 @@ where ); diff_event(event, old_event, coord, dom_path, changes); - diff_children(children, old_children, coord, dom_path, changes)?; + if *reset_inner.borrow() { + // children is empty after innerHTML or innerText changed + diff_children(children, &[], coord, dom_path, changes)?; + } else { + diff_children(children, old_children, coord, dom_path, changes)?; + } } } (RespoNode::Referenced(new_cell), RespoNode::Referenced(old_cell)) => { @@ -147,6 +154,7 @@ fn diff_attrs( coord: &[RespoCoord], dom_path: &[u32], changes: &mut Vec>, + reset_inner: &RefCell, ) where T: Debug + Clone, { @@ -156,15 +164,24 @@ fn diff_attrs( if old_attrs.contains_key(key) { if &old_attrs[key] != value { added.insert(key.to_owned(), value.to_owned()); + if inner_changed(key) { + *reset_inner.borrow_mut() = true; + } } } else { added.insert(key.to_owned(), value.to_owned()); + if inner_changed(key) { + *reset_inner.borrow_mut() = true; + } } } for key in old_attrs.keys() { if !new_attrs.contains_key(key) { removed.insert(key.to_owned()); + if inner_changed(key) { + *reset_inner.borrow_mut() = true; + } } } @@ -178,6 +195,11 @@ fn diff_attrs( } } +/// changed innerHTML or innerText, which resets children values +fn inner_changed(key: &Rc) -> bool { + key == &"innerHTML".into() || key == &"innerText".into() || key == &"inner-text".into() +} + fn diff_style( new_style: &HashMap, String>, old_style: &HashMap, String>, diff --git a/respo/src/app/patch.rs b/respo/src/app/patch.rs index 1e42d86..0a389ca 100644 --- a/respo/src/app/patch.rs +++ b/respo/src/app/patch.rs @@ -199,7 +199,10 @@ where .expect("get node") .children() .item(*idx) - .ok_or_else(|| format!("child to remove not found at {}", &idx))?; + .ok_or_else(|| { + util::warn_log!("child not found at {:?}", coord); + format!("child to remove not found at {}", &idx) + })?; target.remove_child(&child).expect("child removed"); } ChildDomOp::InsertAfter(idx, k, node) => { diff --git a/respo/src/app/renderer.rs b/respo/src/app/renderer.rs index 623a3a8..85af86a 100644 --- a/respo/src/app/renderer.rs +++ b/respo/src/app/renderer.rs @@ -3,6 +3,7 @@ use crate::node::dom_change::RespoCoord; use crate::node::{ DispatchFn, DomChange, RespoComponent, RespoEffectType, RespoElement, RespoEventMark, RespoEventMarkFn, RespoListenerFn, RespoNode, }; +use crate::warn_log; use std::cell::RefCell; use std::fmt::Debug; use std::rc::Rc; @@ -10,7 +11,7 @@ use std::sync::RwLock; use wasm_bindgen::{JsCast, JsValue}; use web_sys::console::{error_1, warn_1}; -use web_sys::{HtmlElement, HtmlLabelElement, HtmlTextAreaElement, Node}; +use web_sys::{HtmlElement, HtmlInputElement, HtmlLabelElement, HtmlTextAreaElement, Node}; use crate::app::diff::{collect_effects_outside_in_as, diff_tree}; use crate::app::patch::{attach_event, patch_tree}; @@ -236,20 +237,28 @@ where children, }) => { let element = document.create_element(name)?; + let mut inner_set = false; for (key, value) in attrs { let key = key.as_ref(); match key { "style" => warn_1(&"style is handled outside attrs".into()), - "innerText" => element.dyn_ref::().expect("into html element").set_inner_text(value), - "innerHTML" => element.set_inner_html(value), + "innerText" => { + inner_set = true; + element.dyn_ref::().expect("into html element").set_inner_text(value) + } + "innerHTML" => { + inner_set = true; + element.set_inner_html(value) + } "htmlFor" => element .dyn_ref::() .expect("into label element") .set_html_for(value), - "value" if &**name == "textarea" || &**name == "input" => element + "value" if &**name == "textarea" => element .dyn_ref::() - .expect("into html element") + .expect("into textarea element") .set_value(value), + "value" if &**name == "input" => element.dyn_ref::().expect("into input element").set_value(value), _ => { element.set_attribute(key, value)?; } @@ -258,6 +267,13 @@ where if !style.is_empty() { element.set_attribute("style", &style.to_string())?; } + if inner_set && !children.is_empty() { + warn_log!( + "innerText or innerHTML is set, it's conflicted with children: {} {:?}", + inner_set, + children + ); + } for (k, child) in children { let mut next_coord = coord.to_owned(); next_coord.push(RespoCoord::Key(k.to_owned())); diff --git a/respo/src/app/util.rs b/respo/src/app/util.rs index 1a1c203..94eab96 100644 --- a/respo/src/app/util.rs +++ b/respo/src/app/util.rs @@ -41,7 +41,7 @@ pub fn raf_loop_slow(interval: i32, mut cb: Box Result<(), String *g.borrow_mut() = Some(Closure::wrap(Box::new(move || { if let Err(e) = cb() { crate::warn_log!( - "Failure in slow loop, program has to stop since inconsistent states. Details: {}", + "Failure in slow loop, program has to stop since inconsistent DOM states. Details: {}", e ); }