Skip to content

Commit

Permalink
fix innerText issues in diffing; tag 0.1.13
Browse files Browse the repository at this point in the history
  • Loading branch information
tiye committed Aug 3, 2024
1 parent 7880f6b commit c6b0d90
Show file tree
Hide file tree
Showing 8 changed files with 117 additions and 12 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

57 changes: 57 additions & 0 deletions demo_respo/src/inner_text.rs
Original file line number Diff line number Diff line change
@@ -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<RespoElement<ActionOp>, String> {
let cursor = states.path();

let state = states.cast_branch::<InnerTextState>();

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")])
}]),
]),
)
}
9 changes: 8 additions & 1 deletion demo_respo/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
extern crate console_error_panic_hook;

mod counter;
mod inner_text;
mod panel;
mod plugins;
mod store;
Expand All @@ -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;
Expand Down Expand Up @@ -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(),
)
Expand Down
2 changes: 1 addition & 1 deletion respo/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
26 changes: 24 additions & 2 deletions respo/src/app/diff.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::cell::RefCell;
use std::collections::{HashMap, HashSet};
use std::fmt::Debug;
use std::rc::Rc;
Expand Down Expand Up @@ -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()),
Expand All @@ -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)) => {
Expand Down Expand Up @@ -147,6 +154,7 @@ fn diff_attrs<T>(
coord: &[RespoCoord],
dom_path: &[u32],
changes: &mut Vec<DomChange<T>>,
reset_inner: &RefCell<bool>,
) where
T: Debug + Clone,
{
Expand All @@ -156,15 +164,24 @@ fn diff_attrs<T>(
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;
}
}
}

Expand All @@ -178,6 +195,11 @@ fn diff_attrs<T>(
}
}

/// changed innerHTML or innerText, which resets children values
fn inner_changed(key: &Rc<str>) -> bool {
key == &"innerHTML".into() || key == &"innerText".into() || key == &"inner-text".into()
}

fn diff_style<T>(
new_style: &HashMap<Rc<str>, String>,
old_style: &HashMap<Rc<str>, String>,
Expand Down
5 changes: 4 additions & 1 deletion respo/src/app/patch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
26 changes: 21 additions & 5 deletions respo/src/app/renderer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ 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;
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};
Expand Down Expand Up @@ -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::<HtmlElement>().expect("into html element").set_inner_text(value),
"innerHTML" => element.set_inner_html(value),
"innerText" => {
inner_set = true;
element.dyn_ref::<HtmlElement>().expect("into html element").set_inner_text(value)
}
"innerHTML" => {
inner_set = true;
element.set_inner_html(value)
}
"htmlFor" => element
.dyn_ref::<HtmlLabelElement>()
.expect("into label element")
.set_html_for(value),
"value" if &**name == "textarea" || &**name == "input" => element
"value" if &**name == "textarea" => element
.dyn_ref::<HtmlTextAreaElement>()
.expect("into html element")
.expect("into textarea element")
.set_value(value),
"value" if &**name == "input" => element.dyn_ref::<HtmlInputElement>().expect("into input element").set_value(value),
_ => {
element.set_attribute(key, value)?;
}
Expand All @@ -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()));
Expand Down
2 changes: 1 addition & 1 deletion respo/src/app/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub fn raf_loop_slow(interval: i32, mut cb: Box<dyn FnMut() -> 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
);
}
Expand Down

0 comments on commit c6b0d90

Please sign in to comment.