Skip to content

Commit

Permalink
Block props update during hydration (#2665)
Browse files Browse the repository at this point in the history
* Delay props.

* Fix next sibling change not synced.

* Use shifting instead.

* Update docs, minor adjustments.

* More predictable props update.

* Delay longer.

* Only delay props during hydration.
  • Loading branch information
futursolo committed May 23, 2022
1 parent a6df894 commit 027ab6a
Show file tree
Hide file tree
Showing 4 changed files with 175 additions and 38 deletions.
124 changes: 94 additions & 30 deletions packages/yew/src/html/component/lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,9 @@ pub(crate) trait Stateful {

fn as_any(&self) -> &dyn Any;
fn as_any_mut(&mut self) -> &mut dyn Any;

#[cfg(feature = "hydration")]
fn mode(&self) -> RenderMode;
}

impl<COMP> Stateful for CompStateInner<COMP>
Expand All @@ -180,6 +183,11 @@ where
self.context.link().clone().into()
}

#[cfg(feature = "hydration")]
fn mode(&self) -> RenderMode {
self.context.mode
}

fn flush_messages(&mut self) -> bool {
self.context
.link()
Expand All @@ -198,7 +206,7 @@ where
};

if self.context.props != props {
self.context.props = Rc::clone(&props);
self.context.props = props;
self.component.changed(&self.context)
} else {
false
Expand All @@ -221,6 +229,8 @@ pub(crate) struct ComponentState {

#[cfg(feature = "csr")]
has_rendered: bool,
#[cfg(feature = "hydration")]
pending_props: Option<Rc<dyn Any>>,

suspension: Option<Suspension>,

Expand Down Expand Up @@ -263,6 +273,8 @@ impl ComponentState {

#[cfg(feature = "csr")]
has_rendered: false,
#[cfg(feature = "hydration")]
pending_props: None,

comp_id,
}
Expand Down Expand Up @@ -303,9 +315,9 @@ impl<COMP: BaseComponent> Runnable for CreateRunner<COMP> {

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

#[cfg(feature = "csr")]
Expand All @@ -318,43 +330,86 @@ impl Runnable for PropsUpdateRunner {
} = *self;

if let Some(state) = shared_state.borrow_mut().as_mut() {
let schedule_render = 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)
if let Some(next_sibling) = next_sibling {
// When components are updated, their siblings were likely also updated
// We also need to shift the bundle so next sibling will be synced to child
// components.
match state.render_state {
#[cfg(feature = "csr")]
ComponentRenderState::Render {
next_sibling: ref mut current_next_sibling,
ref parent,
ref bundle,
..
} => {
bundle.shift(parent, next_sibling.clone());
*current_next_sibling = next_sibling;
}

#[cfg(feature = "hydration")]
ComponentRenderState::Hydration {
next_sibling: ref mut current_next_sibling,
ref parent,
ref fragment,
..
} => {
fragment.shift(parent, next_sibling.clone());
*current_next_sibling = next_sibling;
}

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

#[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)
}
let should_render = |props: Option<Rc<dyn Any>>, state: &mut ComponentState| -> bool {
props.map(|m| state.inner.props_changed(m)).unwrap_or(false)
};

#[cfg(feature = "ssr")]
ComponentRenderState::Ssr { .. } => {
#[cfg(debug_assertions)]
panic!("properties do not change during SSR");
#[cfg(feature = "hydration")]
let should_render_hydration =
|props: Option<Rc<dyn Any>>, state: &mut ComponentState| -> bool {
if let Some(props) = props.or_else(|| state.pending_props.take()) {
match state.has_rendered {
true => {
state.pending_props = None;
state.inner.props_changed(props)
}
false => {
state.pending_props = Some(props);
false
}
}
} else {
false
}
};

#[cfg(not(debug_assertions))]
false
// Only trigger changed if props were changed / next sibling has changed.
let schedule_render = {
#[cfg(feature = "hydration")]
{
if state.inner.mode() == RenderMode::Hydration {
should_render_hydration(props, state)
} else {
should_render(props, state)
}
}

#[cfg(not(feature = "hydration"))]
should_render(props, state)
};

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

if schedule_render {
Expand Down Expand Up @@ -621,6 +676,15 @@ mod feat_csr {
if state.suspension.is_none() {
state.inner.rendered(self.first_render);
}

#[cfg(feature = "hydration")]
if state.pending_props.is_some() {
scheduler::push_component_props_update(Box::new(PropsUpdateRunner {
props: None,
state: self.state.clone(),
next_sibling: None,
}));
}
}
}
}
Expand Down
9 changes: 9 additions & 0 deletions packages/yew/src/html/component/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,15 @@ impl<COMP: BaseComponent> Context<COMP> {
///
/// We provide a blanket implementation of this trait for every member that implements
/// [`Component`].
///
/// # Warning
///
/// This trait may be subject to heavy changes between versions and is not intended for direct
/// implementation.
///
/// You should used the [`Component`] trait or the
/// [`#[function_component]`](crate::functional::function_component) macro to define your
/// components.
pub trait BaseComponent: Sized + 'static {
/// The Component's Message.
type Message: 'static;
Expand Down
4 changes: 2 additions & 2 deletions packages/yew/src/html/component/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -487,8 +487,8 @@ mod feat_csr {
) {
scheduler::push_component_props_update(Box::new(PropsUpdateRunner {
state,
next_sibling,
props,
next_sibling: Some(next_sibling),
props: Some(props),
}));
// Not guaranteed to already have the scheduler started
scheduler::start();
Expand Down

1 comment on commit 027ab6a

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yew master branch benchmarks (Lower is better)

Benchmark suite Current: 027ab6a Previous: a6df894 Ratio
yew-struct-keyed 01_run1k 204.1505 239.447 0.85
yew-struct-keyed 02_replace1k 210.018 258.82849999999996 0.81
yew-struct-keyed 03_update10th1k_x16 421.48 451.1065 0.93
yew-struct-keyed 04_select1k 85.28450000000001 69.7575 1.22
yew-struct-keyed 05_swap1k 108.018 101.831 1.06
yew-struct-keyed 06_remove-one-1k 35.575 35.967 0.99
yew-struct-keyed 07_create10k 3420.9745000000003 3938.0945 0.87
yew-struct-keyed 08_create1k-after1k_x2 482.12 597.098 0.81
yew-struct-keyed 09_clear1k_x8 205.782 244.885 0.84
yew-struct-keyed 21_ready-memory 1.4598236083984375 1.4598236083984375 1
yew-struct-keyed 22_run-memory 1.6948318481445312 1.6972427368164062 1.00
yew-struct-keyed 23_update5-memory 1.7120361328125 1.699481964111328 1.01
yew-struct-keyed 24_run5-memory 1.7175521850585938 1.7169418334960938 1.00
yew-struct-keyed 25_run-clear-memory 1.3321037292480469 1.3326072692871094 1.00
yew-struct-keyed 31_startup-ci 1732.672 1733.7600000000002 1.00
yew-struct-keyed 32_startup-bt 31.40799999999999 35.239999999999995 0.89
yew-struct-keyed 33_startup-mainthreadcost 261.19199999999995 322.53999999999996 0.81
yew-struct-keyed 34_startup-totalbytes 331.59765625 331.59765625 1

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.