Skip to content

Commit 02d9e81

Browse files
committed
fix(yew): defer rendered in Suspense until children's DOM is live
Components that suspend inside <Suspense> have their DOM parked in a detached parent until the Suspense unsuspends. Previously, when a child resumed from suspension, its `rendered` (and therefore use_effect) fired before the scheduler got to run the Suspense's update+reconcile that shifts the DOM into the live tree, so effects observed detached or missing DOM. Instead of scheduling `rendered` on the scheduler queue directly, a resuming child now hands its RenderedRunner to its ancestor BaseSuspense as a PendingRendered, keyed by component id. BaseSuspense drains this map in its own `rendered` lifecycle, but only once its suspension list is empty: that is the commit on which reconcile has already shifted children from the detached parent into the live tree. Re-committed children merge against their existing entry so `first_render=true` is preserved. This also correctly handles the sibling case: if A resumes while B is still pending, A's rendered waits until B also resumes and the whole boundary un-suspends, so both effects observe live DOM together. Fixes #3780
1 parent 28219ae commit 02d9e81

4 files changed

Lines changed: 293 additions & 10 deletions

File tree

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

Lines changed: 66 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,8 @@ impl ComponentState {
487487
fn commit_render(&mut self, shared_state: &Shared<Option<ComponentState>>, new_vdom: Html) {
488488
// Currently not suspended, we remove any previous suspension and update
489489
// normally.
490+
#[cfg(feature = "csr")]
491+
let resuming_from_suspension = self.suspension.is_some();
490492
self.resume_existing_suspension();
491493

492494
match self.render_state {
@@ -508,14 +510,28 @@ impl ComponentState {
508510
let first_render = !self.has_rendered;
509511
self.has_rendered = true;
510512

511-
scheduler::push_component_rendered(
512-
self.comp_id,
513-
Box::new(RenderedRunner {
514-
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+
}),
515532
first_render,
516-
}),
517-
first_render,
518-
);
533+
);
534+
}
519535
}
520536

521537
#[cfg(feature = "hydration")]
@@ -701,6 +717,48 @@ mod feat_csr {
701717
pub first_render: bool,
702718
}
703719

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+
/// Merge two pending rendereds for the same component, preserving
737+
/// `first_render=true` if either carried it.
738+
pub(crate) fn merge(self, other: Self) -> Self {
739+
Self {
740+
state: self.state,
741+
first_render: self.first_render || other.first_render,
742+
}
743+
}
744+
745+
/// Push this onto the scheduler's `rendered` queue.
746+
pub(crate) fn schedule(self, comp_id: usize) {
747+
let PendingRendered {
748+
state,
749+
first_render,
750+
} = self;
751+
scheduler::push_component_rendered(
752+
comp_id,
753+
Box::new(RenderedRunner {
754+
state,
755+
first_render,
756+
}),
757+
false,
758+
);
759+
}
760+
}
761+
704762
impl ComponentState {
705763
#[tracing::instrument(
706764
level = tracing::Level::DEBUG,
@@ -741,7 +799,7 @@ mod feat_csr {
741799
}
742800

743801
#[cfg(feature = "csr")]
744-
pub(super) use feat_csr::*;
802+
pub(crate) use feat_csr::*;
745803

746804
#[cfg(all(target_arch = "wasm32", not(target_os = "wasi")))]
747805
#[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: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,14 @@ 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+
#[cfg(feature = "csr")]
20+
use std::collections::BTreeMap;
21+
1722
use super::*;
23+
#[cfg(feature = "csr")]
24+
use crate::html::PendingRendered;
1825
use crate::html::{Component, Context, Html, Scope};
1926
use crate::suspense::Suspension;
2027
#[cfg(feature = "hydration")]
@@ -35,11 +42,25 @@ mod feat_csr_ssr {
3542
Resume(Suspension),
3643
}
3744

38-
#[derive(Debug)]
3945
pub(crate) struct BaseSuspense {
4046
suspensions: Vec<Suspension>,
4147
#[cfg(feature = "hydration")]
4248
hydration_handle: Option<SuspensionHandle>,
49+
/// Rendered runners for child components that resumed while this
50+
/// Suspense was still suspended (because of other pending siblings).
51+
/// Drained in `rendered` once the Suspense fully un-suspends, so
52+
/// effects fire only after children's DOM has been shifted into the
53+
/// live tree.
54+
#[cfg(feature = "csr")]
55+
pending_rendered: RefCell<BTreeMap<usize, PendingRendered>>,
56+
}
57+
58+
impl std::fmt::Debug for BaseSuspense {
59+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
60+
f.debug_struct("BaseSuspense")
61+
.field("suspensions", &self.suspensions)
62+
.finish()
63+
}
4364
}
4465

4566
impl Component for BaseSuspense {
@@ -73,6 +94,8 @@ mod feat_csr_ssr {
7394
suspensions,
7495
#[cfg(feature = "hydration")]
7596
hydration_handle,
97+
#[cfg(feature = "csr")]
98+
pending_rendered: RefCell::new(BTreeMap::new()),
7699
}
77100
}
78101

@@ -128,13 +151,25 @@ mod feat_csr_ssr {
128151
}
129152
}
130153

131-
#[cfg(feature = "hydration")]
132154
fn rendered(&mut self, _ctx: &Context<Self>, first_render: bool) {
155+
#[cfg(not(feature = "hydration"))]
156+
let _ = first_render;
157+
#[cfg(feature = "hydration")]
133158
if first_render {
134159
if let Some(m) = self.hydration_handle.take() {
135160
m.resume();
136161
}
137162
}
163+
// Fire deferred rendered callbacks for children that resumed while
164+
// we were still suspended. Only safe now that we're un-suspended:
165+
// the last reconcile shifted their DOM into the live tree.
166+
#[cfg(feature = "csr")]
167+
if self.suspensions.is_empty() {
168+
let pending = std::mem::take(&mut *self.pending_rendered.borrow_mut());
169+
for (comp_id, p) in pending {
170+
p.schedule(comp_id);
171+
}
172+
}
138173
}
139174
}
140175

@@ -146,6 +181,28 @@ mod feat_csr_ssr {
146181
pub(crate) fn resume(scope: &Scope<Self>, s: Suspension) {
147182
scope.send_message(BaseSuspenseMsg::Resume(s));
148183
}
184+
185+
/// Queue a child component's `rendered` lifecycle to be scheduled once
186+
/// this Suspense fully un-suspends and its reconcile has shifted the
187+
/// child's DOM into the live tree. If the child already has a pending
188+
/// entry (e.g. it re-committed between suspensions), the two are
189+
/// merged so `first_render=true` is not lost.
190+
#[cfg(feature = "csr")]
191+
pub(crate) fn defer_rendered(
192+
scope: &Scope<Self>,
193+
comp_id: usize,
194+
pending: PendingRendered,
195+
) {
196+
let Some(comp) = scope.get_component() else {
197+
return;
198+
};
199+
let mut q = comp.pending_rendered.borrow_mut();
200+
let entry = match q.remove(&comp_id) {
201+
Some(prev) => pending.merge(prev),
202+
None => pending,
203+
};
204+
q.insert(comp_id, entry);
205+
}
149206
}
150207

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

packages/yew/tests/suspense.rs

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -816,3 +816,169 @@ async fn test_duplicate_suspension() {
816816
let result = obtain_result();
817817
assert_eq!(result.as_str(), "hello!");
818818
}
819+
820+
// Regression test for https://github.com/yewstack/yew/issues/3780
821+
// use_future causes use_effect to fire before DOM is updated, so
822+
// document.get_element_by_id returns None for elements the component renders.
823+
#[wasm_bindgen_test]
824+
async fn use_effect_can_access_dom_after_use_future_resolves() {
825+
#[derive(Properties, Clone)]
826+
struct ContentProps {
827+
dom_observed: Rc<RefCell<Option<bool>>>,
828+
}
829+
830+
impl PartialEq for ContentProps {
831+
fn eq(&self, _other: &Self) -> bool {
832+
true
833+
}
834+
}
835+
836+
#[component(Content)]
837+
fn content(props: &ContentProps) -> HtmlResult {
838+
use_future(|| async {
839+
sleep(Duration::ZERO).await;
840+
})?;
841+
842+
{
843+
let dom_observed = props.dom_observed.clone();
844+
use_effect_with((), move |_| {
845+
let element = gloo::utils::document().get_element_by_id("foo");
846+
*dom_observed.borrow_mut() = Some(element.is_some());
847+
|| {}
848+
});
849+
}
850+
851+
Ok(html! {
852+
<div id="result">
853+
<div id="foo"></div>
854+
</div>
855+
})
856+
}
857+
858+
#[derive(Properties, Clone)]
859+
struct AppProps {
860+
dom_observed: Rc<RefCell<Option<bool>>>,
861+
}
862+
863+
impl PartialEq for AppProps {
864+
fn eq(&self, _other: &Self) -> bool {
865+
true
866+
}
867+
}
868+
869+
#[component(App)]
870+
fn app(props: &AppProps) -> Html {
871+
html! {
872+
<Suspense fallback={html! {<div>{"loading"}</div>}}>
873+
<Content dom_observed={props.dom_observed.clone()} />
874+
</Suspense>
875+
}
876+
}
877+
878+
let dom_observed: Rc<RefCell<Option<bool>>> = Rc::new(RefCell::new(None));
879+
880+
yew::Renderer::<App>::with_root_and_props(
881+
gloo::utils::document().get_element_by_id("output").unwrap(),
882+
AppProps {
883+
dom_observed: dom_observed.clone(),
884+
},
885+
)
886+
.render();
887+
888+
// After everything settles (suspension resolves, component renders, effects run),
889+
// use_effect should have found the #foo element in the DOM.
890+
//
891+
// Bug (issue #3780): use_effect fires before the DOM is updated when use_future
892+
// is involved, so get_element_by_id("foo") returns None and dom_observed is Some(false).
893+
// Expected: use_effect fires after the DOM is committed, so dom_observed should be
894+
// Some(true).
895+
sleep(Duration::from_millis(50)).await;
896+
assert_eq!(
897+
*dom_observed.borrow(),
898+
Some(true),
899+
"use_effect should see the rendered DOM element after use_future resolves"
900+
);
901+
}
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)