Skip to content

Commit

Permalink
xilem_web: Fix DomChildrenSplice::with_scratch (#484)
Browse files Browse the repository at this point in the history
I'm surprised that this was not noticed yet,
`DomChildrenSplice::with_scratch` previously only appended new elements
to the end of the children of an element in the DOM tree, whereas it
should've inserted it at the current index of the `ElementSplice`. This
should fix this, using a `DocumentFragment` which is shared on the
`ViewCtx` to avoid unnecessary allocations.
  • Loading branch information
Philipp-M authored Aug 5, 2024
1 parent 0bca54a commit 544a4a1
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 6 deletions.
1 change: 1 addition & 0 deletions xilem_web/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ features = [
"console",
"CssStyleDeclaration",
"Document",
"DocumentFragment",
"DomTokenList",
"Element",
"Event",
Expand Down
12 changes: 11 additions & 1 deletion xilem_web/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,20 @@ impl MessageThunk {
}

/// The [`View`](`crate::core::View`) `Context` which is used for all [`DomView`](`crate::DomView`)s
#[derive(Default)]
pub struct ViewCtx {
id_path: Vec<ViewId>,
app_ref: Option<Box<dyn AppRunner>>,
pub(crate) fragment: Rc<web_sys::DocumentFragment>,
}

impl Default for ViewCtx {
fn default() -> Self {
ViewCtx {
id_path: Vec::default(),
app_ref: None,
fragment: Rc::new(crate::document().create_document_fragment()),
}
}
}

impl ViewCtx {
Expand Down
23 changes: 18 additions & 5 deletions xilem_web/src/elements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@

//! Basic builder functions to create DOM elements, such as [`html::div`]
use std::any::Any;
use std::borrow::Cow;
use std::{any::Any, rc::Rc};
use wasm_bindgen::{JsCast, UnwrapThrowExt};

use crate::{
Expand Down Expand Up @@ -135,6 +135,7 @@ pub struct DomChildrenSplice<'a, 'b, 'c, 'd> {
children: VecSplice<'b, 'c, AnyPod>,
ix: usize,
parent: &'d web_sys::Node,
fragment: Rc<web_sys::DocumentFragment>,
parent_was_removed: bool,
}

Expand All @@ -144,13 +145,15 @@ impl<'a, 'b, 'c, 'd> DomChildrenSplice<'a, 'b, 'c, 'd> {
children: &'b mut Vec<AnyPod>,
vec_splice_scratch: &'c mut Vec<AnyPod>,
parent: &'d web_sys::Node,
fragment: Rc<web_sys::DocumentFragment>,
parent_was_deleted: bool,
) -> Self {
Self {
scratch,
children: VecSplice::new(children, vec_splice_scratch),
ix: 0,
parent,
fragment,
parent_was_removed: parent_was_deleted,
}
}
Expand All @@ -159,12 +162,20 @@ impl<'a, 'b, 'c, 'd> DomChildrenSplice<'a, 'b, 'c, 'd> {
impl<'a, 'b, 'c, 'd> ElementSplice<AnyPod> for DomChildrenSplice<'a, 'b, 'c, 'd> {
fn with_scratch<R>(&mut self, f: impl FnOnce(&mut AppendVec<AnyPod>) -> R) -> R {
let ret = f(self.scratch);
for element in self.scratch.drain() {
if !self.scratch.is_empty() {
for element in self.scratch.drain() {
self.fragment
.append_child(element.node.as_ref())
.unwrap_throw();
self.children.insert(element);
self.ix += 1;
}
self.parent
.append_child(element.node.as_ref())
.insert_before(
self.fragment.as_ref(),
self.children.next_mut().map(|p| p.node.as_ref()),
)
.unwrap_throw();
self.children.insert(element);
self.ix += 1;
}
ret
}
Expand Down Expand Up @@ -262,6 +273,7 @@ where
&mut element.props.children,
&mut state.vec_splice_scratch,
element.node.as_ref(),
ctx.fragment.clone(),
element.was_removed,
);
children.dyn_seq_rebuild(
Expand Down Expand Up @@ -290,6 +302,7 @@ pub(crate) fn teardown_element<State, Action, Element, SeqMarker>(
&mut element.props.children,
&mut state.vec_splice_scratch,
element.node.as_ref(),
ctx.fragment.clone(),
true,
);
children.dyn_seq_teardown(&mut state.seq_state, ctx, &mut dom_children_splice);
Expand Down

0 comments on commit 544a4a1

Please sign in to comment.