Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delay Hydration second render until all assistive nodes have been removed #2629

Merged
merged 22 commits into from
Apr 24, 2022
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/js_callback/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ license = "MIT OR Apache-2.0"

[dependencies]
wasm-bindgen = "0.2"
yew = { path = "../../packages/yew", features = ["csr"] }
yew = { path = "../../packages/yew", features = ["csr", "tokio"] }
ranile marked this conversation as resolved.
Show resolved Hide resolved
wasm-bindgen-futures = "0.4"
js-sys = "0.3"
once_cell = "1"
25 changes: 7 additions & 18 deletions packages/yew/src/dom_bundle/bcomp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,6 @@ use web_sys::Element;
pub(super) struct BComp {
type_id: TypeId,
scope: Box<dyn Scoped>,
// A internal NodeRef passed around to track this components position. This
// is "stable", i.e. does not change when reconciled.
internal_ref: NodeRef,
// The user-passed NodeRef from VComp. Might change every time we reconcile.
// Gets linked to the internal ref
node_ref: NodeRef,
key: Option<Key>,
}
Expand Down Expand Up @@ -62,23 +57,20 @@ impl Reconcilable for VComp {
node_ref,
key,
} = self;
let internal_ref = NodeRef::default();
node_ref.link(internal_ref.clone());

let scope = mountable.mount(
root,
internal_ref.clone(),
node_ref.clone(),
parent_scope,
parent.to_owned(),
next_sibling,
);

(
internal_ref.clone(),
node_ref.clone(),
BComp {
type_id,
node_ref,
internal_ref,
key,
scope,
},
Expand Down Expand Up @@ -120,10 +112,10 @@ impl Reconcilable for VComp {
} = self;

bcomp.key = key;
let old_ref = std::mem::replace(&mut bcomp.node_ref, node_ref);
let old_ref = std::mem::replace(&mut bcomp.node_ref, node_ref.clone());
bcomp.node_ref.reuse(old_ref);
mountable.reuse(bcomp.scope.borrow(), next_sibling);
bcomp.internal_ref.clone()
mountable.reuse(node_ref.clone(), bcomp.scope.borrow(), next_sibling);
node_ref
}
}

Expand All @@ -147,24 +139,21 @@ mod feat_hydration {
node_ref,
key,
} = self;
let internal_ref = NodeRef::default();
node_ref.link(internal_ref.clone());

let scoped = mountable.hydrate(
root.clone(),
parent_scope,
parent.clone(),
fragment,
internal_ref.clone(),
node_ref.clone(),
);

(
internal_ref.clone(),
node_ref.clone(),
BComp {
type_id,
scope: scoped,
node_ref,
internal_ref,
key,
},
)
Expand Down
2 changes: 1 addition & 1 deletion packages/yew/src/dom_bundle/blist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ mod feat_hydration {
let (child_node_ref, child) = child.hydrate(root, parent_scope, parent, fragment);

if index == 0 {
node_ref.link(child_node_ref);
node_ref.reuse(child_node_ref);
}

children.push(child);
Expand Down
5 changes: 1 addition & 4 deletions packages/yew/src/dom_bundle/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@ pub(super) fn insert_node(node: &Node, parent: &Element, next_sibling: Option<&N
match next_sibling {
Some(next_sibling) => parent
.insert_before(node, Some(next_sibling))
.unwrap_or_else(|err| {
gloo::console::error!("failed to insert node", err, parent, next_sibling, node);
panic!("failed to insert tag before next sibling")
}),
.expect("failed to insert tag before next sibling"),
None => parent.append_child(node).expect("failed to append child"),
};
}
Expand Down
188 changes: 119 additions & 69 deletions packages/yew/src/html/component/lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ use crate::scheduler::{self, Runnable, Shared};
use crate::suspense::{BaseSuspense, Suspension};
use crate::{Callback, Context, HtmlResult};
use std::any::Any;
#[cfg(feature = "csr")]
use std::cell::Cell;
use std::rc::Rc;

#[cfg(feature = "hydration")]
Expand Down Expand Up @@ -219,7 +221,7 @@ pub(crate) struct ComponentState {
pub(super) render_state: ComponentRenderState,

#[cfg(feature = "csr")]
has_rendered: bool,
has_rendered: Rc<Cell<bool>>,

suspension: Option<Suspension>,

Expand Down Expand Up @@ -261,7 +263,7 @@ impl ComponentState {
suspension: None,

#[cfg(feature = "csr")]
has_rendered: false,
has_rendered: Rc::default(),

comp_id,
}
Expand Down Expand Up @@ -300,62 +302,98 @@ impl<COMP: BaseComponent> Runnable for CreateRunner<COMP> {
}
}

pub(crate) enum UpdateEvent {
/// Drain messages for a component.
Message,
/// Wraps properties, node ref, and next sibling for a component
#[cfg(feature = "csr")]
Properties(Rc<dyn Any>, NodeRef),
}

pub(crate) struct UpdateRunner {
#[cfg(feature = "csr")]
pub(crate) struct PropsUpdateRunner {
pub props: Rc<dyn Any>,
pub state: Shared<Option<ComponentState>>,
pub event: UpdateEvent,
pub node_ref: NodeRef,
pub next_sibling: NodeRef,
}

impl Runnable for UpdateRunner {
#[cfg(feature = "csr")]
impl Runnable for PropsUpdateRunner {
fn run(self: Box<Self>) {
if let Some(state) = self.state.borrow_mut().as_mut() {
let schedule_render = match self.event {
UpdateEvent::Message => state.inner.flush_messages(),
let Self {
node_ref: next_node_ref,
next_sibling,
props,
state: shared_state,
} = *self;

if let Some(state) = shared_state.borrow_mut().as_mut() {
let schedule_render = match state.render_state {
#[cfg(feature = "csr")]
UpdateEvent::Properties(props, next_sibling) => {
match state.render_state {
#[cfg(feature = "csr")]
ComponentRenderState::Render {
next_sibling: ref mut current_next_sibling,
..
} => {
// When components are updated, their siblings were likely also updated
*current_next_sibling = next_sibling;
// Only trigger changed if props were changed
state.inner.props_changed(props)
}

#[cfg(feature = "hydration")]
ComponentRenderState::Hydration {
next_sibling: ref mut current_next_sibling,
..
} => {
// When components are updated, their siblings were likely also updated
*current_next_sibling = next_sibling;
// Only trigger changed if props were changed
state.inner.props_changed(props)
}

#[cfg(feature = "ssr")]
ComponentRenderState::Ssr { .. } => {
#[cfg(debug_assertions)]
panic!("properties do not change during SSR");

#[cfg(not(debug_assertions))]
false
}
ComponentRenderState::Render {
ref mut node_ref,
next_sibling: ref mut current_next_sibling,
..
} => {
// When components are updated, a new node ref could have been passed in
if *node_ref != next_node_ref {
node_ref.set(None);
*node_ref = next_node_ref;
}
// When components are updated, their siblings were likely also updated
*current_next_sibling = next_sibling;
// Only trigger changed if props were changed
state.inner.props_changed(props)
}

#[cfg(feature = "hydration")]
ComponentRenderState::Hydration {
ref mut node_ref,
next_sibling: ref mut current_next_sibling,
..
} => {
// When components are updated, a new node ref could have been passed in
if *node_ref != next_node_ref {
node_ref.set(None);
*node_ref = next_node_ref;
}
// When components are updated, their siblings were likely also updated
*current_next_sibling = next_sibling;
// Only trigger changed if props were changed
state.inner.props_changed(props)
}

#[cfg(feature = "ssr")]
ComponentRenderState::Ssr { .. } => {
#[cfg(debug_assertions)]
panic!("properties do not change during SSR");

#[cfg(not(debug_assertions))]
false
}
};

#[cfg(debug_assertions)]
super::log_event(
state.comp_id,
format!("props_update(schedule_render={})", schedule_render),
);

if schedule_render {
scheduler::push_component_render(
state.comp_id,
Box::new(RenderRunner {
state: shared_state.clone(),
}),
);
// Only run from the scheduler, so no need to call `scheduler::start()`
}
};
}
}

pub(crate) struct UpdateRunner {
pub state: Shared<Option<ComponentState>>,
}

impl Runnable for UpdateRunner {
fn run(self: Box<Self>) {
if let Some(state) = self.state.borrow_mut().as_mut() {
let schedule_render = state.inner.flush_messages();

#[cfg(debug_assertions)]
super::log_event(
state.comp_id,
Expand Down Expand Up @@ -450,15 +488,38 @@ impl RenderRunner {

let comp_id = state.comp_id;

if suspension.resumed() {
// schedule a render immediately if suspension is resumed.
#[cfg(feature = "csr")]
let schedule_render = {
let has_rendered = state.has_rendered.clone();
move || {
if has_rendered.get() {
scheduler::push_component_render(
comp_id,
Box::new(RenderRunner {
state: shared_state.clone(),
}),
);
} else {
scheduler::push_component_priority_render(Box::new(RenderRunner {
state: shared_state.clone(),
}));
}
}
};

#[cfg(not(feature = "csr"))]
let schedule_render = move || {
scheduler::push_component_render(
state.comp_id,
comp_id,
Box::new(RenderRunner {
state: shared_state,
state: shared_state.clone(),
}),
);
};

if suspension.resumed() {
// schedule a render immediately if suspension is resumed.
schedule_render();
} else {
// We schedule a render after current suspension is resumed.
let comp_scope = state.inner.any_scope();
Expand All @@ -468,15 +529,7 @@ impl RenderRunner {
.expect("To suspend rendering, a <Suspense /> component is required.");
let suspense = suspense_scope.get_component().unwrap();

suspension.listen(Callback::from(move |_| {
scheduler::push_component_render(
comp_id,
Box::new(RenderRunner {
state: shared_state.clone(),
}),
);
scheduler::start();
}));
suspension.listen(Callback::from(move |_| schedule_render()));

if let Some(ref last_suspension) = state.suspension {
if &suspension != last_suspension {
Expand Down Expand Up @@ -518,8 +571,8 @@ impl RenderRunner {
bundle.reconcile(root, &scope, parent, next_sibling.clone(), new_root);
node_ref.link(new_node_ref);

let first_render = !state.has_rendered;
state.has_rendered = true;
let first_render = !state.has_rendered.get();
state.has_rendered.set(true);

scheduler::push_component_rendered(
state.comp_id,
Expand All @@ -541,12 +594,9 @@ impl RenderRunner {
} => {
// We schedule a "first" render to run immediately after hydration,
// to fix NodeRefs (first_node and next_sibling).
scheduler::push_component_first_render(
state.comp_id,
Box::new(RenderRunner {
state: self.state.clone(),
}),
);
scheduler::push_component_priority_render(Box::new(RenderRunner {
state: self.state.clone(),
}));

let scope = state.inner.any_scope();

Expand Down