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

feat: implement some signal traits for tuples #3649

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

QuartzLibrary
Copy link

This implements some signal-related traits for tuples.

  • To to satisfy the ReadUntracked::Value: Deref bound and the Clone bound in the ReadUntracked -> WithUntracked -> GetUntracked blanket implementations I had to use a couple of wrappers.
    • This does mean that things like .with will not be perfect, requiring for example two *s to get to the value in some cases.
  • I am not sure what to do about Set, and whether to just remove it. I don't think there is as much of an use case for it anyway.
    • Options:
      • The current implementation always panics if setting partially fails.
      • Alternatives are to always panic, or never panic.
      • Finally another alternative is to have SetUntracked::Value be a tuple of options so that the try variant can partially return the values.
    • I am leaning toward removing it.

Example expansion
impl<T0, T1> Dispose for (T0, T1)
where
    T0: Dispose,
    T1: Dispose,
{
    fn dispose(self) {
        self.0.dispose();
        self.1.dispose();
    }
}
impl<T0, T1> Track for (T0, T1)
where
    T0: Track,
    T1: Track,
{
    fn track(&self) {
        self.0.track();
        self.1.track();
    }
}
impl<T0, T1> DefinedAt for (T0, T1)
where
    T0: DefinedAt,
    T1: DefinedAt,
{
    fn defined_at(&self) -> Option<&'static Location<'static>> {
        let locs = [self.0.defined_at(), self.1.defined_at()];
        crate::log_warning(format_args!(
            "{DEFINED_AT_MSG} All locations: [{}]",
            MaybeLocations { locs: &locs },
        ));
        locs[0]
    }
}
impl<T0, T1> ReadUntracked for (T0, T1)
where
    T0: ReadUntracked,
    T1: ReadUntracked,
{
    type Value = TupleReadWrapper<(
        TupleReadFieldWrapper<T0>,
        TupleReadFieldWrapper<T1>,
    )>;
    fn try_read_untracked(&self) -> Option<Self::Value> {
        Some(TupleReadWrapper((
            TupleReadFieldWrapper::new(&self.0)?,
            TupleReadFieldWrapper::new(&self.1)?,
        )))
    }
}
impl<T0, T1> Notify for (T0, T1)
where
    T0: Notify,
    T1: Notify,
{
    fn notify(&self) {
        self.0.notify();
        self.1.notify();
    }
}
impl<T0, T1> Set for (T0, T1)
where
    T0: Set,
    T1: Set,
{
    type Value = (T0::Value, T1::Value);
    fn set(&self, value: Self::Value) {
        self.0.set(value.0);
        self.1.set(value.1);
    }
    fn try_set(&self, value: Self::Value) -> Option<Self::Value> {
        let values = (self.0.try_set(value.0), self.1.try_set(value.1));
        let all_none = values.0.is_none() && values.1.is_none() && true;
        let all_some = values.0.is_some() && values.1.is_some() && true;
        {
            if !(all_none || all_some) {
                {
                    core::panicking::panic_fmt(core::const_format_args!(
                        "{PARTIAL_SET_MSG}"
                    ));
                };
            }
        };
        Some((values.0?, values.1?))
    }
}
impl<T0, T1> IsDisposed for (T0, T1)
where
    T0: IsDisposed,
    T1: IsDisposed,
{
    fn is_disposed(&self) -> bool {
        self.0.is_disposed() && self.1.is_disposed() && true
    }
}```

</details>

@gbj
Copy link
Collaborator

gbj commented Feb 24, 2025

What's the reasoning/use case here?

@QuartzLibrary
Copy link
Author

Hi, apologies for not clarifying, it's something I've meant to do a while ago but then forgot.

The context is buried somewhere on Discord, but basically it would be really nice with things like .map and .for_each to keep the reactive graph spelled out even when it's not just a single signal. The single-signal version works pretty well, and I have often chained helpers like that to improve code quality/readability significantly.

After opening this PR though, it occurred to me that I should try implementing the extension traits directly on the tuples and see if it works, or it if it causes issues with blanket impls and similar. ~All the utility is in those adapters after all.
With control of the associated types I might even be able to use GATs to avoid the double deref problem.
I'll report back on that.

@gbj
Copy link
Collaborator

gbj commented Mar 3, 2025

Since .map() and .for_each() methods do not exist in leptos, I'm assuming those are part of the extension traits that you're talking about, so yes I'd suggest that you try implementing those directly on tuples, if that's the desired end goal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants