Improve reactive signal accessed outside reactive context warning.

[bicarlsen]

[0.7]

Is your feature request related to a problem? Please describe.
When a signal is accessed outside a reactive context, the current warning displays the location of the signal access and where it was defined

e.g.

At app.rs:112:34, you access a `reactive_graph::signal::read::ReadSignal<bool>` (defined at app.rs:209:27) outside a reactive tracking context.

However, if a signal is accessed in a nested signal (e.g. via Signal::derive) where the nested signal is not in a reactive context, it is still the original signal that is referenced int he warning, not the derived signal.

e.g.

use leptos::{prelude::*, task::spawn};

#[component]
pub fn App() -> impl IntoView {
    view! {
       <HomePage />
    }
}

#[component]
fn HomePage() -> impl IntoView {
    let things = RwSignal::new(vec![
        Thing::new("One"),
        Thing::new("Two"), // (4)
        Thing::new("Three"),
    ]);

    let names = Signal::derive(move || { // (0)
        things
            .read() // (1)
            .iter()
            .map(|thing| thing.name.get())
            .collect::<Vec<_>>()
    });

    spawn(async move { names.get() }); // (2)
}

struct Thing {
    name: ReadSignal<String>,
}

impl Thing {
    fn new(s: &'static str) -> Self {
        Self {
            name: RwSignal::new(s.to_string()).read_only(),
        }
    }
}

Even though it is (2) causing the accessed outside reactive context warning, it is (1) being referenced in the error.

Describe the solution you'd like
The warning should reference the most immediate cause of being outside a reactive context, perhaps allowing for a backtrace to the signal causing the update as well.

e.g.

At `<(2)>`, you access a `reactive_graph::signal::read::ReadSignal<Vec<String>>` (defined at `<(0)>`) outside a reactive tracking context.
Backtrace `<(2)>` <- `<(0)>` <- `<(1)>` <- `<(4)>`

Describe alternatives you've considered
N/A

Additional context
Issue #3354 seems related, but different.

[gbj]

Note that Signal::derive() is just a boxed closure, and does not create a new reactive node; if you do actually create a new reactive node by using Memo::new() here instead, this behaves exactly as you'd expect. Signal::derive behaves exactly like simply using move || { ... } would, because that's basically what it is.

I guess I'm open to the idea that there should be some special behavior here that differentiates Signal::derive(move || { ... }) from move || { ... }, beyond the fact that it's just type-erased.

Is this something you're interested in working on a PR to improve?

[bicarlsen]

Would be happy to give it a shot, but may need a lot of guidance, if that's okay for you.

The first thing that comes to mind would be that any time we get into a "track outside of reactive context" state, we look at the stack frames below until we get back into a reactive context, then report those series of calls as the backtrace.

Not sure how that would work or if it's the correct approach, so would love others' thoughts.

[veigaribo]

I had thought about the defined_at property being a Vec of locations, instead of just a single one. Then, in those sorts of situations where one signal is constructed somehow based on another, its location could be added, instead of either replacing or not replacing the old one.

[bicarlsen]

@gbj, any thoughts on how to go about this before I dive in?

[gbj]

@gbj, any thoughts on how to go about this before I dive in?

I have not spent a lot of time thinking about it.

I think it should be pretty straightforward though, and should not require the more-in-depth work described above.

Signal already has a defined_at field:

leptos/reactive_graph/src/wrappers.rs

Lines 375 to 380 in 28af33d

	pub struct Signal<T, S = SyncStorage>
	where
	S: Storage<T>,
	{
	#[cfg(any(debug_assertions, leptos_debuginfo))]
	defined_at: &'static Location<'static>,

But it doesn't check whether you're in a reactive zone or not in its .track() implementation

https://github.com/leptos-rs/leptos/blob/main/reactive_graph/src/wrappers.rs#L449-L468

As opposed to the (blanket) impl for other signal types, which does that check

https://github.com/leptos-rs/leptos/blob/main/reactive_graph/src/traits.rs#L120-L146

So you'd mostly just want to add, in the SignalTypes::Stored(_) => {} branch, the equivalent of the code above which, in debug mode, checks whether you're in a reactive zone and warns otherwise. (You could also then suppress the warnings for the inner signals with SpecialNonReactiveZone::enter() if it makes the warnings less spammy)