Recursive effects never run after recursing

[Giovanni-Tably]

Describe the bug

Effects that are recursively triggered (set signals they transitively depend on) stop reacting after recursing.
That is, the recursion is prevented somehow, and afterward the effect loses the reactive links to the signals it subscribes to and never runs again.

Also relevant:

• Recursive effects are iffy and best avoided. :D
• The context this came up in was an effect resetting a signal it depended on, which is not great but also a simple and obvious way to implement the intended behaviour.
• This can be fixed:
  • with set_untracked to break the recursion, but only if that effect is the only subscriber, or
  • by setting the signal in a separate execution context with something like queue_microtask. Internally we use a defer_with function you can see here. Note that we use the parent scope and not defer because the issue seemed to persist without it.

Leptos Dependencies

Tested in the leptos repo on 8686d5a.

To Reproduce

Copy the following test in leptos_reactive/tests/effect.rs then run:

cd leptos_reactive
cargo test recursive_effect -- --nocapture

#[test]
fn recursive_effect() {
    use leptos::{SignalGet, SignalGetUntracked, SignalSet};

    let _runtime = create_runtime();

    let s = leptos::create_rw_signal(0);

    leptos::create_isomorphic_effect(move |_| {
        let a = s.get();
        print!("{a}");
        if a == 0 {
            return;
        }
        s.set(0);
    });

    s.set(1);
    s.set(2);
    s.set(3);

    assert_eq!(3, s.get_untracked()); // Passes

    // Current output `01`
}

Expected behavior

I expected the test to print 0123 (if recursion is prevented) or maybe 0102030(if recursion is allowed and is up to the user to manage it). The current output is 01.

[gbj]

I looked into this a bit and I can write up a longer explanation if you'd really like to know; the short version is that it effects are marked "clean" when they finish running, so marking it dirty while it's still dirty doesn't work as you expect (because it then marks itself clean).

I'm struggling to come up with a reason this would be a good idea, though -- could you share a use case?

[Giovanni-Tably]

Thank you for looking into this!

---

I looked into this a bit and I can write up a longer explanation if you'd really like to know; the short version is that it effects are marked "clean" when they finish running, so marking it dirty while it's still dirty doesn't work as you expect (because it then marks itself clean).

Aah, that makes sense. And while marking it dirty again, it also somehow clears the subscriptions the effect is supposed to be picking up?
Though it might also mean the reproduction is not quite what I encountered originally then (more details below).

I'm struggling to come up with a reason this would be a good idea, though

I agree that it's best avoided if possible, especially the simple repro above which does not involve async code. But it also feels like a pattern that is easy to stumble into in simple cases, and tricky to make sure you don't stumble into in involved applications where the recursion could be many layers removed.
In practice debugging it is also tricky, because the first update will often be correct, and any run after that will be silently broken.
On a more subjective note, I personally find the behaviours above simplify the mental model in terms of leptos code behaving as much as possible like 'normal' code, where recursion either works or panics (as in recursive RefCell borrowing).

could you share a use case?

In my case the app/user would be modifying a complex signal that is widely shared, it needs to remain editable and be present in the UI while the edits finish, and then further while an animation runs. When all of that is done, we process the signal and then reset it.

When we wrote the code we had unscoped tasks (as in, a simple wasm_bindgen_futures::spawn_local with cancel-on-drop), which was working as there was no actual recursion in the reactive sense happening.
Then we introduced scoping (ScopedFuture), which triggered the problem (not quite sure why, since it would be executed asynchronously. The owner is still relevant somehow?).
The above is the simplest reproduction I could find, which turned out not to require async code, but maybe there are two separate behaviours?

The actual code would have looked something like:

Desugared

// This is a widely shared signal, bound to the UI in a bunch of places
let something: Signal<Option<_>> = todo!();

// Here we spawn a task to do some work and then clear the signal.
let spawned_task = None; // Let's skip RefCell stuff
create_effect(move |_| {
  drop(task.take());
  
  let fut = async {
    if let Some(v) = something.get() {
      sleep().await;
      do_stuff();
      something.set(None);
    }
  };
  let fut = ScopedFuture::new_current(fut).expect("an owner should be present");
  let fut = async move { fut.await.unwrap() }; // In practice we warn if the task fails.
  
  task = Some(spawn_with_handle(fut));

});

// This is a widely shared signal, bound to the UI in a bunch of places.
let something: Signal<Option<_>> = todo!();

// Here we spawn a task to do something, if re-run the task is reset.
// Uses [ScopedFuture] with the effect as the owner internally.
something.for_each_async(move |v| async {
  if let Some(v) = something.get() {
    sleep().await;
    do_stuff();
    something.set(None);
  }
});

// This code is not quite right (like the timing), but not in relevant-to-the-issue ways.

---

Reproduction including async, pardon the hackiness:

Repro with sync, async, and scoped async

(requires adding tokio as a dev dependency tokio = { version = "1", features = ["rt", "macros"]})

#[test]
fn recursive_effect() {
    use leptos::{SignalGet, SignalGetUntracked, SignalSet};

    let _runtime = create_runtime();

    let s = leptos::create_rw_signal(0);

    leptos::create_isomorphic_effect(move |_| {
        let a = s.get();
        println!("{a}");
        if a == 0 {
            return;
        }
        s.set(0);
    });

    s.set(1);
    s.set(2);
    s.set(3);

    assert_eq!(0, s.get_untracked()); // Different from OP, does not pass

    // Prints:
    // 0
    // 1
    // <panic>
}

#[tokio::test]
async fn recursive_effect_async_unscoped() {
    use std::time::Duration;

    use leptos::{SignalGet, SignalGetUntracked, SignalSet};
    use tokio::time::sleep;

    let _runtime = create_runtime();

    let s = leptos::create_rw_signal(0);

    leptos::create_isomorphic_effect(move |_| {
        let a = s.get();
        println!("sync {a}");
        tokio::spawn(async move {
            println!("async {a}");
            if a == 0 {
                return;
            }
            s.set(0);
        })
    });
    sleep(Duration::from_secs(1)).await;

    s.set(1);
    sleep(Duration::from_secs(1)).await;
    s.set(2);
    sleep(Duration::from_secs(1)).await;
    s.set(3);
    sleep(Duration::from_secs(1)).await;

    assert_eq!(0, s.get_untracked()); // Passes

    // Prints:
    // sync 0
    // async 0
    // sync 1
    // async 1
    // sync 0
    // async 0
    // sync 2
    // async 2
    // sync 0
    // async 0
    // sync 3
    // async 3
    // sync 0
    // async 0
}

#[tokio::test]
async fn recursive_effect_async_scoped() {
    use std::time::Duration;

    use leptos::{ScopedFuture, SignalGet, SignalGetUntracked, SignalSet};
    use tokio::time::sleep;

    let _runtime = create_runtime();

    let s = leptos::create_rw_signal(0);

    leptos::create_isomorphic_effect(move |_| {
        let a = s.get();
        println!("sync {a}");
        let fut = async move {
            println!("async {a}");
            if a == 0 {
                return;
            }
            s.set(0);
        };
        let fut = ScopedFuture::new_current(fut).unwrap();
        let fut = async move { fut.await.unwrap() };
        tokio::spawn(fut);
    });
    sleep(Duration::from_secs(1)).await;

    s.set(1);
    sleep(Duration::from_secs(1)).await;
    s.set(2);
    sleep(Duration::from_secs(1)).await;
    s.set(3);
    sleep(Duration::from_secs(1)).await;

    assert_eq!(0, s.get_untracked()); // Does not pass

    // Prints:
    // sync 0
    // async 0
    // sync 1
    // async 1
    // <panic>
}

---

Whatever way leptos decides to go forward with, I would be happy to PR any required change and/or document recursion behaviour.
I guess with the new reactive system coming up it might also make sense to shelve this temporarily, which would be totally fine.

[luxalpa]

You can somewhat workaround this issue by setting the signal from within a call to queue_microtask

[Giovanni-Tably]

You can somewhat workaround this issue by setting the signal from within a call to queue_microtask

That indeed works and should be ~equivalent to using wasm_bindgen_futures::spawn_local to mimic the reproduction cases above. It'll leak the function though, which can cause panics if the signal is disposed of in the meantime.

I would recommend using something like defer_with to only execute if a parent scope hasn't been cleaned up yet. If you don't use a parent scope then it still won't work. Not sure if it needs to be a parent of the effect, signal, or both. In our case it was both.