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

Use method instead of trait for .call syntax #1921

Closed
wants to merge 3 commits into from

Conversation

rambip
Copy link
Contributor

@rambip rambip commented Oct 21, 2023

This PR completely removes the Callable trait from leptos_reactive, since it was used only in callback.rs and it didn't add anything compared to a method call.

It is technically a breaking change, but only for people that used the Callback trait explicitely. Given that callbacks does not even appear in the book, this will likely affect nobody.

The main motivation for this change is to suppress a compiler warning.

If you were on nightly and used the .call syntax, the compiler would give you a warning saying "you may need to disambiguate .call in the future because a trait method will be called like that". It can be very annoying when someone writes a library for leptos using stable, and someone use it on nightly.

Since method calls take precedence, you will not get any warning with this new version.

I plan to create a chapter about callbacks and generic function props, but the content will depend on whether this PR is accepted

@gbj
Copy link
Collaborator

gbj commented Oct 21, 2023

Happy to schedule this for 0.6, but removing a pub trait is a breaking change so it can't be done before that. "Breaking change" here doesn't mean "is it likely to break a bunch of people's code," it means "can it potentially break a previously-working build?" and the answer is "yes, definitely." If use leptos::Callable; compiles and is necessary in 0.5.1 and doesn't compile in 0.5.2, that's the problem.

@gbj gbj added this to the 0.6 milestone Oct 21, 2023
@rambip
Copy link
Contributor Author

rambip commented Oct 21, 2023

Right

I will make a different PR to add the missing Copy implementation for SyncCallback

I will also add a .invoke method, so that we can make Callable deprecated and guide towards .invoke (if the name is not too bad)

@rambip
Copy link
Contributor Author

rambip commented Oct 23, 2023

I just realized I didn't implement From<S: SignalGet> on non-nightly.

The issue with a lot of structs that implement Fn is that the behaviour (like the From trait) is different between nightly and non-nightly.

Since it will likely become worse in the future, it would be nice to enforce some guidelines to make sure the behaviour is as similar as possible for nightly and stable rust

@gbj gbj modified the milestones: 0.7, 0.6 Jan 4, 2024
@benwis benwis changed the base branch from main to leptos_v0.6 January 15, 2024 18:53
@benwis
Copy link
Contributor

benwis commented Jan 15, 2024

@rambip If you'd like to resolve the conflicts, we can merge this into the 0.6 release branch.

@gbj gbj deleted the branch leptos-rs:leptos_v0.6 January 21, 2024 20:22
@gbj gbj closed this Jan 21, 2024
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.

3 participants