leptos-rs / leptos

Build fast web applications with Rust.
https://leptos.dev
MIT License
16.37k stars 656 forks source link

Intermediate memo does not recompute #3252

Open stefnotch opened 11 hours ago

stefnotch commented 11 hours ago

Describe the bug I have set up a reactive graph with "source => memo 1 => memo 2" and "memo1, memo2 => effect". When I change the source, then the first memo gets recomputed, the effect gets updated, but the second memo stays the same.

use any_spawner::Executor;
use reactive_graph::{
    computed::Memo,
    effect::Effect,
    owner::Owner,
    signal::RwSignal,
    traits::{Get, Read, Set},
};

fn main() {
    Executor::init_futures_executor().unwrap();
    let _owner = Owner::new();
    let source = RwSignal::new(0);

    let directly_derived = Memo::new_with_compare(move |_| source.get(), |_, _| true);
    let indirect = Memo::new_with_compare(move |_| directly_derived.get(), |_, _| true);

    Effect::new(move |_| {
        let direct_value = directly_derived.read(); // <!!!=== This seems to break it
        let indirect_value = indirect.get();
        println!(
            "After rerunning the effect effect {:?} ?= {:?} (memo should have been recomputed ^^^)",
            *direct_value, indirect_value
        );
    });

    Executor::poll_local();
    source.set(1);
    Executor::poll_local();
    source.set(2);
    Executor::poll_local();
}

Leptos Dependencies

[dependencies]
any_spawner = { version = "0.1.1", features = [
    "wasm-bindgen",
    "futures-executor",
] }
reactive_graph = { version = "0.1.0-rc1", features = ["effects"] }
wasm-bindgen = "0.2.95"
web-sys = { version = "0.3.72", features = ["console"] }

[patch.crates-io]
any_spawner = { git = "https://github.com/leptos-rs/leptos", branch = "main" }

To Reproduce Steps to reproduce the behavior:

  1. Run the script above
  2. It prints
    After rerunning the effect effect 0 ?= 0 (memo should have been recomputed ^^^)
    After rerunning the effect effect 1 ?= 0 (memo should have been recomputed ^^^)
    After rerunning the effect effect 2 ?= 0 (memo should have been recomputed ^^^)

Expected behavior It should have printed


After rerunning the effect effect 0 ?= 0 (memo should have been recomputed ^^^)
After rerunning the effect effect 1 ?= 1 (memo should have been recomputed ^^^)
After rerunning the effect effect 2 ?= 2 (memo should have been recomputed ^^^)

Additional context

I also have a demo project that can run in a browser environment leptos-testos.zip

The demo project can also be run in a browser environment by doing

npm install
npm run build

and then serving index.html with a HTTP server and opening it in a browser. I originally thought that it was a wasm-bindgen-futures specific bug, but it turned out that I just didn't manage to properly reproduce it on desktop.

gbj commented 7 hours ago

Thanks for the repro, I noticed this or a similar bug last week but had trouble tracking it down. This is a nice minimal example.

gbj commented 6 hours ago

Ah you know what, this is actually just the opposite of #3158 which you opened a while ago. Now, rather than deadlocking, having a ReadLock on a memo while it needs to be updated is failing silently.

Using .get() instead of .read() fixes the issue.

I am not sure that it is possible to refactor the way memos are structured such that the value and the reactive machinery are separate from one another, because whether a memo needs to update or not depends on its value. So taking a read lock on a memo, and holding that during other reactive operations, is quite tricky.

Any suggestions? Perhaps the answer is just a better answer to errors here, like a console warning instead of either panicking or failing silently?

stefnotch commented 5 hours ago

An error or a warning would be a very welcome improvement indeed. 👍

As for what I'm doing: I'm using memos with objects that cannot be cloned. For example, GPU shader code is in a signal, and a wgpu::ShaderModule is in a memo that is derived from the signal. With the ReadGuard API I can get a reference to such an object.

In my case, I can also rework my code to use .with(|value| { do something with the value }), it's just a lot of extra nested code. Alternatively, I could wrap everything in an Arc, so that the objects are clone-able.

So there's a part of me that very much hopes for a world where the .read() API almost always works.

stefnotch commented 5 hours ago

Another tidbit worth mentioning: I have quite a lot of memos that always recompute their value when their inputs change. And they never bother comparing the new value with the old value, because they cannot be the same. For example Memo::new_with_compare(move |_| createShaderModule(sourceCode.get()), |_, _| true);

gbj commented 4 hours ago

And they never bother comparing the new value with the old value, because they cannot be the same. For example Memo::new_with_compare(move |_| createShaderModule(sourceCode.get()), |_, _| true);

That's fine, this is just not a memo and should be implemented differently. In fact, because of the trait-based approach in reactive_graph I'm pretty sure it's possible to create your own primitive, in userland, that does not have the same limitations you're seeing with a memo.

On the other hand, I'm not sure what purpose a memo is serving here? If the memos are defined such that they are never the same value, it's a bunch of reactive-graph overhead for no real reason, as far as I can tell.

stefnotch commented 4 hours ago

I used a memo, because it seemed like the most straightforward way of doing "[...] will only run once per change, no matter how many times you access its value.", while also having laziness.

I suppose I could write my own primitive, although I'm not entirely sure where to start.