Skip to content

Commit 4dcca09

Browse files
committed
fix: move deferred rendered off the scheduler onto BaseSuspense
The previous approach routed a resuming child's rendered through the scheduler's `rendered` queue, which drains only after the update+render queues. That worked for a single suspender but not for siblings: if A resumed while B was still pending, A's rendered still fired before the boundary un-suspended, because Suspense's render with suspended=true still doesn't shift children into the live tree. Hand the pending rendered directly to the ancestor BaseSuspense as a PendingRendered, keyed by comp_id in a Vec. BaseSuspense drains this list in its own rendered lifecycle, gated on self.suspensions.is_empty() - i.e. the commit on which reconcile has already shifted children from the detached parent into the live tree. Re-committed children absorb into the existing entry so first_render=true survives a suspend/resume/suspend/resume cycle.
1 parent b3897ff commit 4dcca09

4 files changed

Lines changed: 209 additions & 16 deletions

File tree

packages/yew/src/html/component/lifecycle.rs

Lines changed: 63 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -510,20 +510,28 @@ impl ComponentState {
510510
let first_render = !self.has_rendered;
511511
self.has_rendered = true;
512512

513-
// When resuming from suspension, the component's DOM nodes live in a
514-
// detached parent. The Suspense ancestor must process the Resume
515-
// message and re-render to move children into the live DOM before
516-
// effects run. Use the `rendered` queue (not `rendered_first`) so
517-
// the scheduler processes the Suspense update and render first.
518-
let schedule_as_first = first_render && !resuming_from_suspension;
519-
scheduler::push_component_rendered(
520-
self.comp_id,
521-
Box::new(RenderedRunner {
522-
state: shared_state.clone(),
513+
if resuming_from_suspension {
514+
// The DOM we just reconciled still lives in the ancestor
515+
// Suspense's detached parent. The Suspense must process
516+
// the Resume message and re-render to shift children into
517+
// the live tree before our effects observe the DOM.
518+
// Hand the pending `rendered` to the Suspense; it will
519+
// re-schedule it once it fully un-suspends.
520+
let pending = PendingRendered::new(shared_state.clone(), first_render);
521+
let suspense_scope = scope
522+
.find_parent_scope::<BaseSuspense>()
523+
.expect("a resuming component must have a Suspense ancestor");
524+
BaseSuspense::defer_rendered(&suspense_scope, self.comp_id, pending);
525+
} else {
526+
scheduler::push_component_rendered(
527+
self.comp_id,
528+
Box::new(RenderedRunner {
529+
state: shared_state.clone(),
530+
first_render,
531+
}),
523532
first_render,
524-
}),
525-
schedule_as_first,
526-
);
533+
);
534+
}
527535
}
528536

529537
#[cfg(feature = "hydration")]
@@ -709,6 +717,47 @@ mod feat_csr {
709717
pub first_render: bool,
710718
}
711719

720+
/// A `rendered` lifecycle deferred by the ancestor `<Suspense>` so it fires
721+
/// only after Suspense un-suspends and children's DOM has been shifted into
722+
/// the live tree. See `BaseSuspense::defer_rendered`.
723+
pub(crate) struct PendingRendered {
724+
pub state: Shared<Option<ComponentState>>,
725+
pub first_render: bool,
726+
}
727+
728+
impl PendingRendered {
729+
pub(crate) fn new(state: Shared<Option<ComponentState>>, first_render: bool) -> Self {
730+
Self {
731+
state,
732+
first_render,
733+
}
734+
}
735+
736+
/// Absorb a later-committed pending rendered for the same component:
737+
/// keep the latest state but preserve `first_render=true` if either
738+
/// side carried it.
739+
pub(crate) fn absorb(&mut self, later: Self) {
740+
self.state = later.state;
741+
self.first_render |= later.first_render;
742+
}
743+
744+
/// Push this onto the scheduler's `rendered` queue.
745+
pub(crate) fn schedule(self, comp_id: usize) {
746+
let PendingRendered {
747+
state,
748+
first_render,
749+
} = self;
750+
scheduler::push_component_rendered(
751+
comp_id,
752+
Box::new(RenderedRunner {
753+
state,
754+
first_render,
755+
}),
756+
false,
757+
);
758+
}
759+
}
760+
712761
impl ComponentState {
713762
#[tracing::instrument(
714763
level = tracing::Level::DEBUG,
@@ -749,7 +798,7 @@ mod feat_csr {
749798
}
750799

751800
#[cfg(feature = "csr")]
752-
pub(super) use feat_csr::*;
801+
pub(crate) use feat_csr::*;
753802

754803
#[cfg(all(target_arch = "wasm32", not(target_os = "wasi")))]
755804
#[cfg(test)]

packages/yew/src/html/component/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ mod scope;
1010
use std::rc::Rc;
1111

1212
pub use children::*;
13+
#[cfg(feature = "csr")]
14+
pub(crate) use lifecycle::PendingRendered;
1315
pub use marker::*;
1416
pub use properties::*;
1517
#[cfg(feature = "csr")]

packages/yew/src/suspense/component.rs

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,12 @@ pub struct SuspenseProps {
1414

1515
#[cfg(any(feature = "csr", feature = "ssr"))]
1616
mod feat_csr_ssr {
17+
#[cfg(feature = "csr")]
18+
use std::cell::RefCell;
19+
1720
use super::*;
21+
#[cfg(feature = "csr")]
22+
use crate::html::PendingRendered;
1823
use crate::html::{Component, Context, Html, Scope};
1924
use crate::suspense::Suspension;
2025
#[cfg(feature = "hydration")]
@@ -35,11 +40,29 @@ mod feat_csr_ssr {
3540
Resume(Suspension),
3641
}
3742

38-
#[derive(Debug)]
3943
pub(crate) struct BaseSuspense {
4044
suspensions: Vec<Suspension>,
4145
#[cfg(feature = "hydration")]
4246
hydration_handle: Option<SuspensionHandle>,
47+
/// Rendered runners for child components that resumed while this
48+
/// Suspense was still suspended (because of other pending siblings).
49+
/// Drained in `rendered` once the Suspense fully un-suspends, so
50+
/// effects fire only after children's DOM has been shifted into the
51+
/// live tree.
52+
///
53+
/// A small `Vec` is used over a map; the expected population is the
54+
/// number of suspending direct descendants resumed in one boundary
55+
/// transition, typically just a handful.
56+
#[cfg(feature = "csr")]
57+
pending_rendered: RefCell<Vec<(usize, PendingRendered)>>,
58+
}
59+
60+
impl std::fmt::Debug for BaseSuspense {
61+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
62+
f.debug_struct("BaseSuspense")
63+
.field("suspensions", &self.suspensions)
64+
.finish()
65+
}
4366
}
4467

4568
impl Component for BaseSuspense {
@@ -73,6 +96,8 @@ mod feat_csr_ssr {
7396
suspensions,
7497
#[cfg(feature = "hydration")]
7598
hydration_handle,
99+
#[cfg(feature = "csr")]
100+
pending_rendered: RefCell::new(Vec::new()),
76101
}
77102
}
78103

@@ -128,13 +153,25 @@ mod feat_csr_ssr {
128153
}
129154
}
130155

131-
#[cfg(feature = "hydration")]
132156
fn rendered(&mut self, _ctx: &Context<Self>, first_render: bool) {
157+
#[cfg(not(feature = "hydration"))]
158+
let _ = first_render;
159+
#[cfg(feature = "hydration")]
133160
if first_render {
134161
if let Some(m) = self.hydration_handle.take() {
135162
m.resume();
136163
}
137164
}
165+
// Fire deferred rendered callbacks for children that resumed while
166+
// we were still suspended. Only safe now that we're un-suspended:
167+
// the last reconcile shifted their DOM into the live tree.
168+
#[cfg(feature = "csr")]
169+
if self.suspensions.is_empty() {
170+
let pending = std::mem::take(&mut *self.pending_rendered.borrow_mut());
171+
for (comp_id, p) in pending {
172+
p.schedule(comp_id);
173+
}
174+
}
138175
}
139176
}
140177

@@ -146,6 +183,28 @@ mod feat_csr_ssr {
146183
pub(crate) fn resume(scope: &Scope<Self>, s: Suspension) {
147184
scope.send_message(BaseSuspenseMsg::Resume(s));
148185
}
186+
187+
/// Queue a child component's `rendered` lifecycle to be scheduled once
188+
/// this Suspense fully un-suspends and its reconcile has shifted the
189+
/// child's DOM into the live tree. If the child already has a pending
190+
/// entry (e.g. it re-committed between suspensions), the two are
191+
/// merged so `first_render=true` is not lost.
192+
#[cfg(feature = "csr")]
193+
pub(crate) fn defer_rendered(
194+
scope: &Scope<Self>,
195+
comp_id: usize,
196+
pending: PendingRendered,
197+
) {
198+
let Some(comp) = scope.get_component() else {
199+
return;
200+
};
201+
let mut q = comp.pending_rendered.borrow_mut();
202+
if let Some(slot) = q.iter_mut().find(|(id, _)| *id == comp_id) {
203+
slot.1.absorb(pending);
204+
} else {
205+
q.push((comp_id, pending));
206+
}
207+
}
149208
}
150209

151210
/// Suspend rendering and show a fallback UI until the underlying task completes.

packages/yew/tests/suspense.rs

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -899,3 +899,86 @@ async fn use_effect_can_access_dom_after_use_future_resolves() {
899899
"use_effect should see the rendered DOM element after use_future resolves"
900900
);
901901
}
902+
903+
// Companion to #3780: when multiple siblings under the same <Suspense> each
904+
// suspend independently and resume at different times, every child's effect
905+
// must see the DOM in the live tree, not the detached parent where Suspense
906+
// parks children while at least one of them is still pending.
907+
#[wasm_bindgen_test]
908+
async fn sibling_suspensions_effects_see_dom() {
909+
#[derive(Properties, Clone)]
910+
struct ChildProps {
911+
id: &'static str,
912+
observed: Rc<RefCell<Vec<(&'static str, bool)>>>,
913+
}
914+
915+
impl PartialEq for ChildProps {
916+
fn eq(&self, other: &Self) -> bool {
917+
self.id == other.id
918+
}
919+
}
920+
921+
#[component(Child)]
922+
fn child(props: &ChildProps) -> HtmlResult {
923+
use_future(|| async {
924+
sleep(Duration::ZERO).await;
925+
})?;
926+
927+
let id = props.id;
928+
let observed = props.observed.clone();
929+
use_effect_with((), move |_| {
930+
let found = gloo::utils::document().get_element_by_id(id).is_some();
931+
observed.borrow_mut().push((id, found));
932+
|| {}
933+
});
934+
935+
Ok(html! { <div id={id}></div> })
936+
}
937+
938+
#[derive(Properties, Clone)]
939+
struct AppProps {
940+
observed: Rc<RefCell<Vec<(&'static str, bool)>>>,
941+
}
942+
943+
impl PartialEq for AppProps {
944+
fn eq(&self, _other: &Self) -> bool {
945+
true
946+
}
947+
}
948+
949+
#[component(App)]
950+
fn app(props: &AppProps) -> Html {
951+
html! {
952+
<Suspense fallback={html! { <div>{"loading"}</div> }}>
953+
<Child id="sib-a" observed={props.observed.clone()} />
954+
<Child id="sib-b" observed={props.observed.clone()} />
955+
</Suspense>
956+
}
957+
}
958+
959+
let observed: Rc<RefCell<Vec<(&'static str, bool)>>> = Rc::new(RefCell::new(Vec::new()));
960+
yew::Renderer::<App>::with_root_and_props(
961+
gloo::utils::document().get_element_by_id("output").unwrap(),
962+
AppProps {
963+
observed: observed.clone(),
964+
},
965+
)
966+
.render();
967+
968+
sleep(Duration::from_millis(100)).await;
969+
970+
let observed = observed.borrow();
971+
assert_eq!(
972+
observed.len(),
973+
2,
974+
"both sibling effects should fire exactly once: {:?}",
975+
*observed
976+
);
977+
for (id, found) in observed.iter() {
978+
assert!(
979+
*found,
980+
"effect for {} did not see DOM in live tree: {:?}",
981+
id, *observed
982+
);
983+
}
984+
}

0 commit comments

Comments
 (0)