Skip to content

Commit 4d85a60

Browse files
committed
refactor(cache): Refactor the Effect and Memo types
- Removed generic closures to save code space - Removed the IEffect trait
1 parent 2b0fbeb commit 4d85a60

File tree

6 files changed

+63
-106
lines changed

6 files changed

+63
-106
lines changed

cache/src/effect.rs

Lines changed: 47 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -4,64 +4,27 @@ use crate::effect_stack::{effect_peak, effect_pop, effect_push};
44

55
/// A reactive effect that runs a closure whenever its dependencies change.
66
///
7-
/// `Effect<F>` behaves similarly to an "event listener" or a callback,
7+
/// `Effect` behaves similarly to an "event listener" or a callback,
88
/// but it is automatically tied to any signals or memos it reads during execution.
99
/// When those dependencies change, the effect will re-run.
1010
///
11-
/// Note: The closure runs **immediately upon creation** via `Effect::wrap`,
11+
/// Note: The closure runs **immediately upon creation** via [`Effect::new`],
1212
/// so the effect is always initialized with an up-to-date value.
1313
///
1414
/// In short:
15-
/// - Like a callback: wraps a closure of type `F` and runs it.
15+
/// - Like a callback: wraps a closure and runs it.
1616
/// - Adds tracking: automatically re-runs when dependent signals change.
1717
/// - Runs once immediately at creation.
18-
///
19-
/// # Type Parameters
20-
///
21-
/// - `F`: The closure type wrapped by this effect. Must implement `Fn()`.
22-
/// The closure is executed immediately upon creation and tracked for reactive updates.
23-
pub struct Effect<F>
24-
where
25-
F: Fn() + 'static,
26-
{
27-
f: F,
18+
pub struct Effect {
19+
f: Box<dyn Fn()>,
2820
}
2921

30-
impl<F> Effect<F>
31-
where
32-
F: Fn(),
33-
{
34-
fn new_inner<D>(f: F, deps: Option<D>) -> Rc<dyn IEffect>
35-
where
36-
D: Fn() + 'static,
37-
{
38-
let e: Rc<dyn IEffect> = Rc::new(Effect { f });
39-
let w = Rc::downgrade(&e);
40-
41-
// Dependency collection only at creation time
42-
effect_push(w.clone(), true);
43-
if let Some(deps) = &deps {
44-
deps();
45-
} else {
46-
e.run();
47-
}
48-
effect_pop(w.clone(), true);
49-
50-
// If there is an additional dependency initializer,
51-
// the `Effect` needs to be run immediately
52-
// after dependency collection is completed.
53-
if deps.is_some() {
54-
run_untracked(&e);
55-
}
56-
57-
e
58-
}
59-
22+
impl Effect {
6023
/// Creates a new `Effect`, wrapping the provided closure
6124
/// and running it immediately for dependency tracking.
6225
///
63-
/// Returns an `Rc<dyn IEffect>` so the effect can be stored and shared
64-
/// as a non-generic trait object.
26+
/// Returns an `Rc<Effect>` so the effect can be stored and shared
27+
/// as a non-generic type.
6528
///
6629
/// # Examples
6730
///
@@ -80,8 +43,16 @@ where
8043
/// assert_eq!(counter.get(), 1);
8144
/// ```
8245
#[allow(clippy::new_ret_no_self)]
83-
pub fn new(f: F) -> Rc<dyn IEffect> {
84-
Self::new_inner::<fn()>(f, None)
46+
pub fn new(f: impl Fn() + 'static) -> Rc<Effect> {
47+
let e: Rc<Effect> = Rc::new(Effect { f: Box::new(f) });
48+
let w = Rc::downgrade(&e);
49+
50+
// Dependency collection only at creation time
51+
effect_push(w.clone(), true);
52+
e.run();
53+
effect_pop(w.clone(), true);
54+
55+
e
8556
}
8657

8758
/// Creates a new `Effect` with an additional dependency initializer.
@@ -97,8 +68,8 @@ where
9768
/// (e.g. `if`/`match`), and you want to ensure that *all possible branches*
9869
/// have their dependencies tracked on the first run.
9970
///
100-
/// Returns an `Rc<dyn IEffect>` so the effect can be stored and shared
101-
/// as a non-generic trait object.
71+
/// Returns an `Rc<Effect>` so the effect can be stored and shared
72+
/// as a non-generic type.
10273
///
10374
/// # Examples
10475
///
@@ -141,46 +112,42 @@ where
141112
/// COUNTER_set(20);
142113
/// assert_eq!(result.get(), 20);
143114
/// ```
144-
pub fn new_with_deps<D>(f: F, deps: D) -> Rc<dyn IEffect>
145-
where
146-
D: Fn() + 'static,
147-
{
148-
Self::new_inner(f, Some(deps))
115+
pub fn new_with_deps(f: impl Fn() + 'static, deps: impl Fn()) -> Rc<Effect> {
116+
let e: Rc<Effect> = Rc::new(Effect { f: Box::new(f) });
117+
let w = Rc::downgrade(&e);
118+
119+
// Dependency collection only at creation time
120+
effect_push(w.clone(), true);
121+
deps();
122+
effect_pop(w.clone(), true);
123+
124+
// If there is an additional dependency initializer,
125+
// the `Effect` needs to be run immediately
126+
// after dependency collection is completed.
127+
run_untracked(&e);
128+
129+
e
149130
}
150-
}
151131

152-
/// A non-generic trait for reactive effects.
153-
///
154-
/// `IEffect` serves as a type-erased trait for `Effect<F>` instances.
155-
/// By implementing `IEffect`, an `Effect<F>` can be stored as `Rc<dyn IEffect>`
156-
/// regardless of the specific closure type `F`. This allows the reactive system
157-
/// to manage multiple effects uniformly without exposing the generic type.
158-
pub trait IEffect {
159132
/// Runs the effect closure.
160133
///
161134
/// Typically called by the reactive system when dependencies change.
162135
///
163136
/// # Notes
164137
///
165-
/// Any calls to `Effect` must be handled with care.
138+
/// After initialization, any call to an `Effect` must go through `run()`.
139+
/// Since the preconditions for executing `run()` differ depending on context
140+
/// (e.g. dependency collection vs. signal-triggered updates), such calls
141+
/// must be handled with care.
166142
///
167-
/// After initialization, any calls to an `Effect` are completely dependent on
168-
/// run() , including Signal-triggered runs or dependency collection. However,
169-
/// these calls have different assumptions.
143+
/// Dependency collection for an `Effect` should be limited to its directly
144+
/// connected signals. The intended call chain is:
170145
///
171-
/// Specifically, dependency collection for an `Effect` should be limited to
172-
/// its directly connected Signals. In this case, the `Effect`'s call chain
173-
/// conforms to the `Effect → Memo(s) → Signal(s)` model, which assumes that
174-
/// the `Effect` must be the start of the call chain. Any Signals linked to
175-
/// calls to other `Effect`s should not be collected, and runs triggered by
176-
/// `Signal`s should not be subject to dependency collection.
177-
fn run(&self);
178-
}
179-
180-
impl<F> IEffect for Effect<F>
181-
where
182-
F: Fn(),
183-
{
146+
/// `Effect → Memo(s) → Signal(s)`
147+
///
148+
/// In this model, the `Effect` must always be the root of the chain.
149+
/// Other `Effect`s should not be tracked as dependencies, and runs triggered
150+
/// by signals should not themselves cause further dependency collection.
184151
fn run(&self) {
185152
assert!(
186153
std::ptr::eq(&*effect_peak().unwrap().effect.upgrade().unwrap(), self),
@@ -191,7 +158,7 @@ where
191158
}
192159
}
193160

194-
pub(crate) fn run_untracked(e: &Rc<dyn IEffect>) {
161+
pub(crate) fn run_untracked(e: &Rc<Effect>) {
195162
let w = Rc::downgrade(e);
196163

197164
effect_push(w.clone(), false);

cache/src/effect_stack.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,24 +4,24 @@ use std::rc::Weak;
44

55
use once_cell::unsync::Lazy;
66

7-
use crate::IEffect;
7+
use crate::Effect;
88

99
pub(crate) struct EffectStackEntry {
10-
pub(crate) effect: Weak<dyn IEffect>,
10+
pub(crate) effect: Weak<Effect>,
1111
pub(crate) collecting: bool,
1212
}
1313

1414
static mut EFFECT_STACK: Lazy<Vec<EffectStackEntry>> = Lazy::new(Vec::new);
1515

16-
pub(crate) fn effect_push(effect: Weak<dyn IEffect>, collecting: bool) {
16+
pub(crate) fn effect_push(effect: Weak<Effect>, collecting: bool) {
1717
unsafe { EFFECT_STACK.push(EffectStackEntry { effect, collecting }) }
1818
}
1919

2020
pub(crate) fn effect_peak() -> Option<&'static EffectStackEntry> {
2121
unsafe { EFFECT_STACK.last() }
2222
}
2323

24-
pub(crate) fn effect_pop(effect: Weak<dyn IEffect>, collecting: bool) {
24+
pub(crate) fn effect_pop(effect: Weak<Effect>, collecting: bool) {
2525
let e = unsafe { EFFECT_STACK.pop() }
2626
.expect("`effect_push` and `effect_pop` are called in pairs and should not be empty.");
2727

cache/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ pub mod memo;
88
pub mod signal;
99

1010
pub(crate) use cache::{remove_from_cache, store_in_cache, touch};
11-
pub use effect::{Effect, IEffect};
11+
pub use effect::Effect;
1212
pub use memo::Memo;
1313
pub use signal::Signal;
1414

cache/src/memo.rs

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use crate::{Observable, call_stack, remove_from_cache, store_in_cache, touch};
44

55
/// A memoized reactive computation that caches its result and tracks dependencies.
66
///
7-
/// `Memo<T, F>` behaves similarly to a computed property: it stores the result of a closure
7+
/// `Memo<T>` behaves similarly to a computed property: it stores the result of a closure
88
/// and only recomputes when its dependencies change. Other signals or effects that access
99
/// the memo will automatically be tracked.
1010
///
@@ -15,29 +15,19 @@ use crate::{Observable, call_stack, remove_from_cache, store_in_cache, touch};
1515
/// # Type Parameters
1616
///
1717
/// - `T`: The result type of the computation. Must implement `Clone`.
18-
/// - `F`: The closure type that computes the value. Must implement `Fn() -> T`.
19-
pub struct Memo<T, F>
20-
where
21-
F: Fn() -> T,
22-
{
23-
f: F,
18+
pub struct Memo<T> {
19+
f: Box<dyn Fn() -> T>,
2420
dependents: RefCell<Vec<&'static dyn Observable>>,
2521
}
2622

27-
impl<T, F> Observable for Memo<T, F>
28-
where
29-
F: Fn() -> T,
30-
{
23+
impl<T> Observable for Memo<T> {
3124
fn invalidate(&'static self) {
3225
remove_from_cache(self);
3326
self.dependents.borrow().iter().for_each(|d| d.invalidate());
3427
}
3528
}
3629

37-
impl<T, F> Memo<T, F>
38-
where
39-
F: Fn() -> T,
40-
{
30+
impl<T> Memo<T> {
4131
/// Creates a new `Memo` wrapping the provided closure.
4232
///
4333
/// # Examples
@@ -47,9 +37,9 @@ where
4737
///
4838
/// let memo = Memo::new(|| 10);
4939
/// ```
50-
pub fn new(f: F) -> Self {
40+
pub fn new(f: impl Fn() -> T + 'static) -> Self {
5141
Memo {
52-
f,
42+
f: Box::new(f),
5343
dependents: vec![].into(),
5444
}
5545
}
@@ -64,7 +54,7 @@ where
6454
/// use once_cell::unsync::Lazy;
6555
/// use reactive_cache::Memo;
6656
///
67-
/// static mut MEMO: Lazy<Memo<i32, fn() -> i32>> = Lazy::new(|| Memo::new(|| 5));
57+
/// static mut MEMO: Lazy<Memo<i32>> = Lazy::new(|| Memo::new(|| 5));
6858
/// assert_eq!(unsafe { (*MEMO).get() }, 5);
6959
/// ```
7060
pub fn get(&'static self) -> T

cache/src/signal.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::{
33
rc::Weak,
44
};
55

6-
use crate::{IEffect, Observable, call_stack, effect_stack::EffectStackEntry};
6+
use crate::{Effect, Observable, call_stack, effect_stack::EffectStackEntry};
77

88
/// A reactive signal that holds a value, tracks dependencies, and triggers effects.
99
///
@@ -23,7 +23,7 @@ use crate::{IEffect, Observable, call_stack, effect_stack::EffectStackEntry};
2323
pub struct Signal<T> {
2424
value: RefCell<T>,
2525
dependents: RefCell<Vec<&'static dyn Observable>>,
26-
effects: RefCell<Vec<Weak<dyn IEffect>>>,
26+
effects: RefCell<Vec<Weak<Effect>>>,
2727
}
2828

2929
impl<T> Signal<T> {

macros/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ pub fn memo(_attr: TokenStream, item: TokenStream) -> TokenStream {
241241
}
242242

243243
let ident = format_ident!("{}", ident.to_string().to_uppercase());
244-
let ty = quote! { reactive_cache::Lazy<reactive_cache::Memo<#output_ty, fn() -> #output_ty>> };
244+
let ty = quote! { reactive_cache::Lazy<reactive_cache::Memo<#output_ty>> };
245245
let expr = quote! { reactive_cache::Lazy::new(|| reactive_cache::Memo::new(|| #block)) };
246246

247247
let expanded = quote! {

0 commit comments

Comments
 (0)