DioxusLabs / dioxus

App framework for web, desktop, mobile, and more.
https://dioxuslabs.com
Apache License 2.0
20.44k stars 786 forks source link

use_drop crashing router #3002

Open chungwong opened 3 days ago

chungwong commented 3 days ago

Problem

Demo: drawer branch in this repo

Description

Given the following Drawer component,

#[component]
fn Drawer(open: Signal<bool>) -> Element {
    // set overflow of body according to the state of `open`
    use_effect(move || {
        tracing::info!("drawer effect");
        body_overflow(open());
    });

    // reset overflow to auto when this Drawer is destroyed
    use_drop(move || {
        tracing::info!("dropping");
        body_overflow(false);
    });

    rsx! {
        if open() {
            div {
                "I am a drawer"
            }
        }
    }
}

/// set overflow property on body
fn body_overflow(open: bool) {
    if let Some(window) = web_sys::window() {
        if let Some(document) = window.document() {
            if let Some(body) = document.body() {
                let overflow = if open {
                    "overflow: hidden"
                } else {
                    "overflow: auto"
                };

                let _ = body.set_attribute("style", overflow);
            }
        }
    }
}

use_effect is used to control the overflow property on the html body. use_drop is used to clean up/reset the changes made to overflow.

Starting the app and on http://127.0.0.1:8080, press F5 twice(once is enough for triggering but twice will present you the problem) to refresh the page and you will see the ValueDroppedError. I don't see any difference in my use case compared to the use_drop doc

server: 2024-09-26T04:43:41.330901Z  INFO myrust: dropping
server: note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
server: 2024-09-26T04:43:41.331014Z  WARN dioxus_signals::warnings::__copy_value_hoisted: A Copy Value created in ScopeId(0) (at /home/chung/.cargo/registry/src/index.crates.io-6f17d22bba15001f/dioxus-router-0.6.0-alpha.2/src/contexts/router.rs:164:20). It will be dropped when that scope is dropped, but it was used in ScopeId(5, "dioxus_router::components::outlet::Outlet<myrust::Route>") (at /home/chung/.cargo/registry/src/index.crates.io-6f17d22bba15001f/dioxus-router-0.6.0-alpha.2/src/contexts/router.rs:303:20) which is not a descendant of the owning scope.
server: This may cause reads or writes to fail because the value is dropped while it still held.
server: thread '<unnamed>' panicked at /home/chung/.cargo/registry/src/index.crates.io-6f17d22bba15001f/dioxus-router-0.6.0-alpha.2/src/contexts/router.rs:303:20:
server: called `Result::unwrap()` on an `Err` value: Dropped(ValueDroppedError { created_at: Location { file: "/home/chung/.cargo/registry/src/index.crates.io-6f17d22bba15001f/dioxus-router-0.6.0-alpha.2/src/contexts/router.rs", line: 164, col: 20 } })
server: stack backtrace:
server: Help:
server: Copy values (like CopyValue, Signal, Memo, and Resource) are owned by the scope they are created in. If you use the value in a scope that may be dropped after the origin scope,
server: it is very easy to use the value after it has been dropped. To fix this, you can move the value to the parent of all of the scopes that it is used in.
server: Broken example ❌:
server: ```rust
server: #[component]
server: fn Counters() -> Element {
server:     let counts = use_signal(Vec::new);
server:     let mut children = use_signal(|| 0);
server:     rsx! {
server:         button { onclick: move |_| children += 1, "Add child" }
server:         button { onclick: move |_| children -= 1, "Remove child" }
server:         // A signal from a child is read or written to in a parent scope
server:         "{counts:?}"
server:         for _ in 0..children() {
server:             Counter {
server:                 counts
server:             }
server:         }
server:     }
server: }
server: #[component]
server: fn Counter(mut counts: Signal<Vec<Signal<i32>>>) -> Element {
server:     let mut signal_owned_by_child = use_signal(|| 0);
server:     // Moving the signal up to the parent may cause issues if you read the signal after the child scope is dropped
server:     use_hook(|| counts.push(signal_owned_by_child));
server:     rsx! {
server:         button {
server:             onclick: move |_| signal_owned_by_child += 1,
server:             "{signal_owned_by_child}"
server:         }
server:     }
server: }
server: ```
server: Fixed example ✅:
server: ```rust
server: #[component]
server: fn Counters() -> Element {
server:     let mut counts = use_signal(Vec::new);
server:     rsx! {
server:         button { onclick: move |_| counts.write().push(0), "Add child" }
server:         button {
server:             onclick: move |_| {
server:                 counts.write().pop();
server:             },
server:             "Remove child"
server:         }
server:         "{counts:?}"
server:         // Instead of passing up a signal, we can just write to the signal that lives in the parent
server:         for index in 0..counts.len() {
server:             Counter {
server:                 index,
server:                 counts
server:             }
server:         }
server:     }
server: }
server: #[component]
server: fn Counter(index: usize, mut counts: Signal<Vec<i32>>) -> Element {
server:     rsx! {
server:         button {
server:             onclick: move |_| counts.write()[index] += 1,
server:             "{counts.read()[index]}"
server:         }
server:     }
server: }
server: ```

Environment:

ealmloff commented 3 days ago

I get a panic related to running wasm-bindgen methods on non-wasm targets first. If you fix that issue by moving the body of the body_overflow function into a web!{} macro (which just passes through the inner code if the target is web), then the other panic goes away as well

chungwong commented 3 days ago

I get a panic related to running wasm-bindgen methods on non-wasm targets first. If you fix that issue by moving the body of the body_overflow function into a web!{} macro (which just passes through the inner code if the target is web), then the other panic goes away as well

Wrapping it in to web! {} did work. However, why is it related though? body_overflow uses web_sys and hence it is web only but use_drop triggered when Drawer get destroyed. Isn't it all web only already?

ealmloff commented 3 days ago

I get a panic related to running wasm-bindgen methods on non-wasm targets first. If you fix that issue by moving the body of the body_overflow function into a web!{} macro (which just passes through the inner code if the target is web), then the other panic goes away as well

Wrapping it in to web! {} did work. However, why is it related though? body_overflow uses web_sys and hence it is web only but use_drop triggered when Drawer get destroyed. Isn't it all web only already?

Effects don't run on the server, but use_drop does. It runs any time the component is dropped including during SSR rendering on the server. My guess is trying to recover from wasm bindgen error caused an odd state on the server which caused the error you saw