Skip to content

Commit b53017d

Browse files
committed
Fix stale state in callbacks when multiple events fire rapidly (#3796)
The UseReducerHandle was caching a snapshot of state at render time. When set() was called, the underlying RefCell was updated, but callback closures still held handles pointing to old snapshots. This caused stale state reads when multiple events fired before a rerender. The fix changes UseReducerHandle to: 1. Store both a reference to the shared RefCell AND a snapshot 2. Use try_borrow() to read from RefCell when possible 3. Fall back to snapshot when RefCell is borrowed (during dispatch) This ensures callbacks see the latest state in normal operation while avoiding panics when state is read during reducer execution (e.g., SSR). Changes: - UseReducerHandle now stores both current_state RefCell ref and snapshot - Deref uses try_borrow with fallback to snapshot - Updated Clone, Debug, PartialEq implementations - Added regression test for the stale state scenario
1 parent 9ebbf99 commit b53017d

File tree

3 files changed

+150
-9
lines changed

3 files changed

+150
-9
lines changed

packages/yew/src/functional/hooks/use_reducer.rs

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,11 @@ pub struct UseReducerHandle<T>
3535
where
3636
T: Reducible,
3737
{
38-
value: Rc<T>,
38+
/// Shared mutable state - always contains the latest value
39+
current_state: Rc<RefCell<Rc<T>>>,
40+
/// Snapshot taken at handle creation, used as fallback when RefCell is borrowed
41+
/// (e.g., during dispatch when the reducer is running)
42+
snapshot: Rc<T>,
3943
dispatch: DispatchFn<T>,
4044
}
4145

@@ -63,7 +67,22 @@ where
6367
type Target = T;
6468

6569
fn deref(&self) -> &Self::Target {
66-
&self.value
70+
// Try to get the latest value from the shared RefCell.
71+
// If it's currently borrowed (e.g., during dispatch/reduce), fall back to snapshot.
72+
//
73+
// SAFETY: This is safe because:
74+
// 1. The Rc<T> (either from RefCell or snapshot) is kept alive by our references
75+
// 2. Rc doesn't move its data, so the pointer is stable
76+
// 3. We only hold the borrow briefly to get the pointer
77+
if let Ok(rc_ref) = self.current_state.try_borrow() {
78+
unsafe {
79+
let ptr: *const T = Rc::as_ptr(&*rc_ref);
80+
&*ptr
81+
}
82+
} else {
83+
// RefCell is mutably borrowed (during dispatch), use snapshot
84+
&self.snapshot
85+
}
6786
}
6887
}
6988

@@ -73,7 +92,8 @@ where
7392
{
7493
fn clone(&self) -> Self {
7594
Self {
76-
value: Rc::clone(&self.value),
95+
current_state: Rc::clone(&self.current_state),
96+
snapshot: Rc::clone(&self.snapshot),
7797
dispatch: Rc::clone(&self.dispatch),
7898
}
7999
}
@@ -84,8 +104,13 @@ where
84104
T: Reducible + fmt::Debug,
85105
{
86106
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
107+
let value = if let Ok(rc_ref) = self.current_state.try_borrow() {
108+
format!("{:?}", *rc_ref)
109+
} else {
110+
format!("{:?}", self.snapshot)
111+
};
87112
f.debug_struct("UseReducerHandle")
88-
.field("value", &format!("{:?}", self.value))
113+
.field("value", &value)
89114
.finish()
90115
}
91116
}
@@ -95,7 +120,8 @@ where
95120
T: Reducible + PartialEq,
96121
{
97122
fn eq(&self, rhs: &Self) -> bool {
98-
self.value == rhs.value
123+
// Compare using deref which handles the try_borrow logic
124+
**self == **rhs
99125
}
100126
}
101127

@@ -239,10 +265,15 @@ where
239265
}
240266
});
241267

242-
let value = state.current_state.borrow().clone();
268+
let current_state = state.current_state.clone();
269+
let snapshot = state.current_state.borrow().clone();
243270
let dispatch = state.dispatch.clone();
244271

245-
UseReducerHandle { value, dispatch }
272+
UseReducerHandle {
273+
current_state,
274+
snapshot,
275+
dispatch,
276+
}
246277
}
247278
}
248279

packages/yew/src/functional/hooks/use_state.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ pub struct UseStateHandle<T> {
109109
impl<T: fmt::Debug> fmt::Debug for UseStateHandle<T> {
110110
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
111111
f.debug_struct("UseStateHandle")
112-
.field("value", &format!("{:?}", self.inner.value))
112+
.field("value", &format!("{:?}", **self))
113113
.finish()
114114
}
115115
}
@@ -132,7 +132,7 @@ impl<T> Deref for UseStateHandle<T> {
132132
type Target = T;
133133

134134
fn deref(&self) -> &Self::Target {
135-
&(*self.inner).value
135+
&self.inner.value
136136
}
137137
}
138138

packages/yew/tests/use_state.rs

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,3 +106,113 @@ async fn use_state_eq_works() {
106106
assert_eq!(result.as_str(), "1");
107107
assert_eq!(RENDER_COUNT.load(Ordering::Relaxed), 2);
108108
}
109+
110+
/// Regression test for issue #3796
111+
/// Tests that state handles always read the latest value even when accessed
112+
/// from callbacks before a rerender occurs.
113+
///
114+
/// The bug occurred when:
115+
/// 1. State A is updated via set()
116+
/// 2. State B is updated via set()
117+
/// 3. A callback reads both states before rerender
118+
/// 4. The callback would see stale value for B because the handle was caching a snapshot instead of
119+
/// reading from the RefCell
120+
#[wasm_bindgen_test]
121+
async fn use_state_handles_read_latest_value_issue_3796() {
122+
use std::cell::RefCell;
123+
124+
use gloo::utils::document;
125+
use wasm_bindgen::JsCast;
126+
use web_sys::HtmlElement;
127+
128+
// Shared storage for the values read by the submit handler
129+
thread_local! {
130+
static CAPTURED_VALUES: RefCell<Option<(String, String)>> = const { RefCell::new(None) };
131+
}
132+
133+
#[component(FormComponent)]
134+
fn form_comp() -> Html {
135+
let field_a = use_state(String::new);
136+
let field_b = use_state(String::new);
137+
138+
let update_a = {
139+
let field_a = field_a.clone();
140+
Callback::from(move |_| {
141+
field_a.set("value_a".to_string());
142+
})
143+
};
144+
145+
let update_b = {
146+
let field_b = field_b.clone();
147+
Callback::from(move |_| {
148+
field_b.set("value_b".to_string());
149+
})
150+
};
151+
152+
// This callback reads both states - the bug caused field_b to be stale
153+
let submit = {
154+
let field_a = field_a.clone();
155+
let field_b = field_b.clone();
156+
Callback::from(move |_| {
157+
let a = (*field_a).clone();
158+
let b = (*field_b).clone();
159+
CAPTURED_VALUES.with(|v| {
160+
*v.borrow_mut() = Some((a.clone(), b.clone()));
161+
});
162+
})
163+
};
164+
165+
html! {
166+
<div>
167+
<button id="update-a" onclick={update_a}>{"Update A"}</button>
168+
<button id="update-b" onclick={update_b}>{"Update B"}</button>
169+
<button id="submit" onclick={submit}>{"Submit"}</button>
170+
<div id="result">{format!("a={}, b={}", *field_a, *field_b)}</div>
171+
</div>
172+
}
173+
}
174+
175+
yew::Renderer::<FormComponent>::with_root(document().get_element_by_id("output").unwrap())
176+
.render();
177+
sleep(Duration::ZERO).await;
178+
179+
// Initial state
180+
let result = obtain_result();
181+
assert_eq!(result.as_str(), "a=, b=");
182+
183+
// Click update-a, then update-b, then submit WITHOUT waiting for rerender
184+
// This simulates rapid user interaction (like the Firefox bug in issue #3796)
185+
document()
186+
.get_element_by_id("update-a")
187+
.unwrap()
188+
.unchecked_into::<HtmlElement>()
189+
.click();
190+
191+
document()
192+
.get_element_by_id("update-b")
193+
.unwrap()
194+
.unchecked_into::<HtmlElement>()
195+
.click();
196+
197+
document()
198+
.get_element_by_id("submit")
199+
.unwrap()
200+
.unchecked_into::<HtmlElement>()
201+
.click();
202+
203+
// Now wait for rerenders to complete
204+
sleep(Duration::ZERO).await;
205+
206+
// Check the values captured by the submit handler
207+
// Before the fix, field_b would be empty because the callback captured a stale handle
208+
let captured = CAPTURED_VALUES.with(|v| v.borrow().clone());
209+
assert_eq!(
210+
captured,
211+
Some(("value_a".to_string(), "value_b".to_string())),
212+
"Submit handler should see latest values for both fields"
213+
);
214+
215+
// Also verify the DOM shows correct values after rerender
216+
let result = obtain_result();
217+
assert_eq!(result.as_str(), "a=value_a, b=value_b");
218+
}

0 commit comments

Comments
 (0)