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

Opinion: Signal should not be callable #2447

Closed
jprider63 opened this issue May 24, 2024 · 3 comments
Closed

Opinion: Signal should not be callable #2447

jprider63 opened this issue May 24, 2024 · 3 comments
Labels
documentation Improvements or additions to documentation signals Related to the signals crate

Comments

@jprider63
Copy link

Feature Request

Currently, to retrieve the contents of a Signal, you need to call the Signal struct. For example:

    let signal: Signal<Option<String>> = use_signal(|| None);
    if let Some(s) = signal() {
        rsx!(
            p {
                s
            }
        )
    }

This seems impossible at first since Signal has kind *, but calling something requires kind * -> * in order to typecheck. If I understand correctly, this is possible by abusing the auto dereferencing rules of Rust and implementing Deref with type Target = dyn Fn() -> T. I believe this is an anti-pattern that should not be used by a popular library. I found this extremely confusing and spent significant time 1) figuring out how to retrieve a T (the only mention in Signal's documentation is hidden in the Deref impl) and 2) understanding how this works.

Implement Suggestion

Remove the Deref implementation and implement a separate method (maybe get?) that retrieves the content of a Signal.

@ealmloff
Copy link
Member

The majority of methods for Signal are implemented on the Readable and Writable trait. One of the methods on the Readable trait is cloned which clones the value out of the signal.

This seems more like a docs/discoverability issue than anything else. I agree looking at the docs of Signal, it isn't clear that you can call the struct or use methods from readable and writable. It would be more clear if we included more examples in the signal docs (including calling the signal like a function) and links to the relevant traits (Readable and Writable)

Implementing the Fn trait itself would also be much more clear than Deref, but unfortunately fn_traits has not landed in stable yet.

@ealmloff ealmloff added documentation Improvements or additions to documentation signals Related to the signals crate labels May 24, 2024
@jprider63
Copy link
Author

Thanks, that's good to know about cloned. I tried using read, but that returned a GenerationalRef which I wasn't sure what to do with.

I was unaware of fn_traits. That also seems like a bad idea in my opinion, but that's a separate issue.

@ealmloff
Copy link
Member

The docs were improved in #2460 to reference the readable and writable traits and include more examples

@ealmloff ealmloff closed this as not planned Won't fix, can't repro, duplicate, stale Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation signals Related to the signals crate
Projects
None yet
Development

No branches or pull requests

2 participants