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

callback::NotRawCallback is not implemented for <closure> #2142

Closed
slyons opened this issue Dec 29, 2023 · 7 comments
Closed

callback::NotRawCallback is not implemented for <closure> #2142

slyons opened this issue Dec 29, 2023 · 7 comments
Labels
bug Something isn't working
Milestone

Comments

@slyons
Copy link

slyons commented Dec 29, 2023

Describe the bug
When using Callbacks as demonstrated in the login_with_token_csr_only example I get the following error:

100 | |  ...                       <Login
101 | |  ...                           api=unauthorized_api
102 | |  ...                           on_success=move |api| {
    | |                                ---------- ----------
    | |                                |          |
    | | _______________________________|__________within this `{closure@lift_frontend/src/lib.rs:102:48: 102:58}`
    | ||                               |
    | ||                               required by a bound introduced by this call
103 | || ...                              log::info!("Successfully logged in");
104 | || ...                               authorized_api.update(|v| *v = Some(api));
105 | || ...                               let navigate = use_navigate();
106 | || ...                               navigate(Page::MeasureList.path(), Default::default());
107 | || ...                               fetch_user_info.dispatch(());
108 | || ...                           }
    | ||_______________________________- this tail expression is of type `{closure@lib.rs:102:48}`
109 | |  ...                       />
110 | |  ...                   }
    | |________________________^ within `{closure@lift_frontend/src/lib.rs:102:48: 102:58}`, the trait `callback::NotRawCallback` is not implemented for `(dyn for<'a> std::ops::Fn(&'a ()) -> Pin<Box<(dyn Future<Output = ()> + 'static)>> + 'static)`

And my build is full of messages like:

warning: a method with this name may be added to the standard library in the future
  --> frontend/src/pages/login.rs:31:36
   |
31 |                         on_success.call(res);
   |                                    ^^^^
   |
   = warning: once this associated item is added to the standard library, the ambiguity may cause an error or change in behavior!
   = note: for more information, see issue #48919 <https://github.com/rust-lang/rust/issues/48919>
   = help: call with fully qualified syntax `leptos::Callable::call(...)` to keep using the current method
   = help: add `#![feature(fn_traits)]` to the crate attributes to enable `std::ops::Fn::call`

Leptos Dependencies

For example:

leptos = { version = "0.5.4", features = ["csr", "nightly"] }
leptos_meta = { version = "0.5.4", features = ["csr"] }
leptos_router = { version = "0.5.4", features = ["csr"] }

To Reproduce

  1. Create new crate
  2. Copy code from example into crate
  3. trunk build
@gbj
Copy link
Collaborator

gbj commented Dec 29, 2023

Is this a duplicate of #2041?

@slyons
Copy link
Author

slyons commented Dec 29, 2023

Could very well be! I've done some experimenting and it seems to be caused by my reference to an Action:

let fetch_user_info = create_action(move |_| async move {
        /*
          Does nothing
        }*/
    });

Fails to compile:

let on_success = move |api| {
        //log::info!("Successfully logged in");
        authorized_api.update(|v| *v = Some(api));
        let navigate = use_navigate();
        navigate(Page::MeasureList.path(), Default::default());
        fetch_user_info.dispatch(());
    };

Compiles (or at least generates no errors):

let on_success = move |api| {
        //log::info!("Successfully logged in");
        authorized_api.update(|v| *v = Some(api));
        let navigate = use_navigate();
        navigate(Page::MeasureList.path(), Default::default());
        //fetch_user_info.dispatch(());
    };

@slyons
Copy link
Author

slyons commented Dec 29, 2023

@slyons
Copy link
Author

slyons commented Dec 29, 2023

This is fixed if I switch back to stable Rust and remove the nightly feature

@gbj
Copy link
Collaborator

gbj commented Dec 30, 2023

I've been in the car all afternoon so haven't dug in but I assume the nightly compiler without nightly feature also works (ie, it's the use of the negative_impls feature in the way it's being used in Leptos nightly feature for Callback, not the nightly compiler without the Leptos nightly feature)

@Baptistemontan
Copy link
Contributor

Baptistemontan commented Feb 26, 2024

The root of the problem is the use of negative_impls and auto_traits with closures, auto traits seams to work kind of like Send and Sync, if a member of the type is !Trait then the struct inherits the negative impl, this means this code won't compile:

#![feature(auto_traits)]
#![feature(negative_impls)]

auto trait Foo {}

struct Bar;

impl !Foo for Bar {}

struct BarWrapper(Bar);

fn assert_foo(_: impl Foo) {}

fn test() {
    assert_foo(BarWrapper(Bar)); // <- error
    let bar = Bar;
    assert_foo(move || {
        let _ = bar;
    }); // <- also error
    assert_foo(move || {}); // <- OK
}

Because BarWrapper containing a !Foo field it then itself becomes !Foo. This is the same for a closure, if you move a !Trait in a closure the closure becomes !Trait. this becomes a problem with generic/dynamic types because it can't be known if the are Trait or !Trait so the compiler assume they could be !Trait, thus throwing an error. It's even worst than this, PhantomData<!Trait> is also !Trait even if it does not really contains a !Trait, so you can't move a generic signal in a callback for example.

The only way to fix this problem is to get rid of the negative trait impl, but it's needed to avoid collision for From<Callback> for Callback and From<Fn> for Callback because Callback implement Fn in nightly.

The only other solutions are either specialization, to specialize the From<Callback> for Callback but From<T> for T is NOT marked as default in the std, so specialization won't help.

The other one I can think of is to use the autoderef specialization trick. Don't directly implement Fn for callback, instead make an inner struct that implements it, and implement Deref for Callback for that inner type, it would look something like this:

#![feature(fn_traits)]
#![feature(unboxed_closures)]

use leptos::{store_value, Callable, StoredValue};

pub struct InnerCallback<In: 'static, Out: 'static>(StoredValue<Box<dyn Fn(In) -> Out>>);

impl<In: 'static, Out: 'static> Callable<In, Out> for InnerCallback<In, Out> {
    fn call(&self, input: In) -> Out {
        self.0.with_value(|f| f(input))
    }
}

impl<In, Out> Clone for InnerCallback<In, Out> {
    fn clone(&self) -> Self {
        *self
    }
}

impl<In, Out> Copy for InnerCallback<In, Out> {}

pub struct Callback<In: 'static, Out: 'static = ()>(InnerCallback<In, Out>);

impl<In: 'static, Out: 'static> Callable<In, Out> for Callback<In, Out> {
    fn call(&self, input: In) -> Out {
        Callable::call(&self.0, input)
    }
}

impl<In> fmt::Debug for Callback<In> {
    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
        fmt.write_str("Callback")
    }
}

impl<In, Out> Clone for Callback<In, Out> {
    fn clone(&self) -> Self {
        *self
    }
}

impl<In, Out> Copy for Callback<In, Out> {}

impl<In, Out> Callback<In, Out> {
    /// Creates a new callback from the given function.
    pub fn new<F>(f: F) -> Callback<In, Out>
    where
        F: Fn(In) -> Out + 'static,
    {
        Self(InnerCallback(store_value(Box::new(f))))
    }
}

impl<In, Out> FnOnce<(In,)> for InnerCallback<In, Out> {
    type Output = Out;

    extern "rust-call" fn call_once(self, args: (In,)) -> Self::Output {
        Callable::call(&self, args.0)
    }
}

impl<In, Out> FnMut<(In,)> for InnerCallback<In, Out> {
    extern "rust-call" fn call_mut(&mut self, args: (In,)) -> Self::Output {
        Callable::call(&*self, args.0)
    }
}

impl<In, Out> Fn<(In,)> for InnerCallback<In, Out> {
    extern "rust-call" fn call(&self, args: (In,)) -> Self::Output {
        Callable::call(self, args.0)
    }
}

impl<In, Out> Deref for Callback<In, Out> {
    type Target = InnerCallback<In, Out>;

    fn deref(&self) -> &Self::Target {
        &self.0
    }
}

impl<F, In, T, Out> From<F> for Callback<In, Out>
where
    F: Fn(In) -> T + 'static,
    T: Into<Out> + 'static,
{
    fn from(f: F) -> Self {
        Self::new(move |x| f(x).into())
    }
}

I'm not a fan of using Deref like this, it's kind of an anti-pattern, but from what I tested it does seams to fix the problem.

@Baptistemontan
Copy link
Contributor

Baptistemontan commented Feb 27, 2024

Just realised, but it is still a breaking change, as functions expecting a impl Fn(In) -> Out can't accept a Callback anymore as it doesn't explicitly implement it...

just need to deref it but still not very convenient:

fn foo() {
    fn inner(_: impl Fn(())) {}
    let cb: Callback<()> = Callback::new(|()| {});
    inner(*cb);
}

@gbj gbj added this to the 0.7 milestone Apr 2, 2024
@gbj gbj added the bug Something isn't working label Apr 2, 2024
gbj added a commit that referenced this issue Aug 10, 2024
gbj added a commit that referenced this issue Aug 12, 2024
@gbj gbj closed this as completed Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants