Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Creating a component causes signals in props to run #1952

Closed
luxalpa opened this issue Oct 26, 2023 · 3 comments · Fixed by #2081
Closed

Creating a component causes signals in props to run #1952

luxalpa opened this issue Oct 26, 2023 · 3 comments · Fixed by #2081
Labels
bug Something isn't working documentation Improvements or additions to documentation

Comments

@luxalpa
Copy link
Contributor

luxalpa commented Oct 26, 2023

Describe the bug
When a component is created within a view and has a signal as props - even if it does not actually use it - it will run the signal once on component creation.

Leptos Dependencies

leptos = { version = "0.5.2", features = ["nightly"] }
leptos_meta = { version = "0.5.2", features = ["nightly"] }
leptos_actix = { version = "0.5.2", optional = true }
leptos_router = { version = "0.5.2", features = ["nightly"] }
leptos_reactive = { version = "0.5.2", features = ["nightly"] }

To Reproduce
Running the following code, even just on actix server, will cause the log message to show. Furthermore, due to the resource::get in that derived signal, we get a warning in the browser that we are reading from a resource outside of Suspense.

use leptos::*;

#[component]
pub fn App() -> impl IntoView {
    let resource = create_blocking_resource(|| {}, |_| async move { 100 });
    let signal = (move || {
        logging::log!("signal");
        resource.get() // <- this runs
    })
    .into_signal();

    view! {
        <OtherComponent signal/>
    }
}

#[component]
fn OtherComponent(signal: Signal<Option<i32>>) -> impl IntoView {}

Expected behavior
The signal should only run when it being subscribed to in a reactive scope.

Additional context
Running the code with env_logger and RUST_LOG=debug gives the output below, which hints that it might be due to the tracing.

[2023-10-26T14:50:27Z DEBUG tracing::span] provide_context;
[2023-10-26T14:50:27Z DEBUG tracing::span] provide_context;
[2023-10-26T14:50:27Z INFO  tracing::span] <App />;
[2023-10-26T14:50:27Z DEBUG leptos_reactive::resource] create_blocking_resource; ty=i32 signal_ty=()
[2023-10-26T14:50:27Z DEBUG leptos_reactive::effect] Effect::run(); defined_at=C:\Users\Luxalpa\.cargo\registry\src\index.crates.io-6f17d22bba15001f\leptos_reactive-0.5.2\src\resource.rs:244:5 ty=()
[2023-10-26T14:50:27Z INFO  tracing::span] <OtherComponent />;
signal
[2023-10-26T14:50:27Z DEBUG tracing::span] use_context;
[2023-10-26T14:50:27Z DEBUG tracing::span] use_context;
[2023-10-26T14:50:27Z DEBUG leptos_reactive::effect] Effect::run(); defined_at=C:\Users\Luxalpa\.cargo\registry\src\index.crates.io-6f17d22bba15001f\leptos_reactive-0.5.2\src\resource.rs:1309:9 ty=()
[2023-10-26T14:50:27Z INFO  tracing::span] <() />;
[2023-10-26T14:50:27Z INFO  tracing::span] <() />;
[2023-10-26T14:50:27Z INFO  leptos_dom::components] <Component />; name=OtherComponent
[2023-10-26T14:50:27Z INFO  leptos_dom] Node; kind="Component"
[2023-10-26T14:50:27Z INFO  leptos_dom::components] <Component />; name=App
[2023-10-26T14:50:27Z INFO  leptos_dom] Node; kind="Component"

Wrapping the signal in a new-type when passing it as a prop seems to workaround the issue.

pub fn App() -> impl IntoView {
  // ...
    view! {
        <OtherComponent signal=MyParam(signal)/>
    }
}

struct MyParam(Signal<Option<i32>>);

#[component]
fn OtherComponent(signal: MyParam) -> impl IntoView {}
@gbj
Copy link
Collaborator

gbj commented Oct 26, 2023

Thanks. As I said briefly on Discord, this is due to the interaction between two things:

  1. Using the tracing feature emits a tracing span when you render a component, which includes serializing its props (this is useful for things like developing future dev tooling)
    } else {
    quote! {
    ::leptos::leptos_dom::tracing_props![#prop_names];
    }
    },
  2. Serializing a signal is a tracked serialization of its value (this is useful for things like using serde_json::to_string() in a create_effect and having it just work)
    self.get().serialize(serializer)

This is why wrapping it in an unserializable newtype changes the behavior, for what it's worth. It will be emitted to tracing as an unserializable prop.

Note that the read on this signal shouldn't be tracked, as the whole component body (including the props tracing) is lazy and only constructed on .into_view() inside an untracked block

let child = untrack_with_diagnostics(|| children_fn().into_view());

I'm open to ideas for better solutions to this.

@luxalpa
Copy link
Contributor Author

luxalpa commented Oct 27, 2023

Hm, I'm skeptical about point 2. I can imagine a lot of scenarios in which I'd want to serialize in response to a button press or some other kind of event where I'd want to use the untracked value. For dev-tools usage I can see wanting to actually subscribe to the signal instead of just having it be serialized automatically.

Anyway, these are my suggestions:

  1. It would probably be a good idea to mention this behavior in the documentation for the component macro and in the book.
  2. The warning message about reading a Resource outside of Suspense could probably be smart enough to figure out whether it sits in an untracked context and not show there. But I don't know if it has other reasons for why you'd still want to show the warning in other untracked contexts. Maybe it could instead determine if it was in a tracing context and avoid showing there?

@gbj
Copy link
Collaborator

gbj commented Oct 27, 2023

To be clear on 2., you can always opt into untracking by calling untrack() around whatever you do. You can't opt into tracking in the same way. So if you want the choice of either tracked or untracked serialization, the Serialize implementation actually needs to be tracked.

@gbj gbj added documentation Improvements or additions to documentation bug Something isn't working labels Nov 24, 2023
@gbj gbj closed this as completed in #2081 Nov 27, 2023
gbj added a commit that referenced this issue Nov 27, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants