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

Implement Into TextProp for signals #2199

Closed

Conversation

SleeplessOne1917
Copy link
Contributor

I'm surprised the impl on line 1357 didn't already cover these.

@gbj
Copy link
Collaborator

gbj commented Jan 21, 2024

The CI is failing on updated nightly clippy warnings that came from outside your commit -- would you mind rebasing so the tests will run on your own code?

At a glance, I would have to assume that this implementation will conflict on nightly with the implementation for Fn() at line 1357 that you point out. These could be cfg-gated to only be used if that feature is not enabled.

@SleeplessOne1917
Copy link
Contributor Author

I have the most recent changes from main merged into this PR now. Is the cfg gate you have in mind enabling these only if it is not nightly?

@gbj
Copy link
Collaborator

gbj commented Jan 22, 2024

Correct. If you look at any of the failing checks here, they are along the lines of

error[E0119]: conflicting implementations of trait `From<signal_wrappers_read::Signal<_>>` for type `TextProp`
    --> /home/runner/work/leptos/leptos/leptos_reactive/src/signal_wrappers_read.rs:1368:1
     |
1357 | / impl<F, S> From<F> for TextProp
1358 | | where
1359 | |     F: Fn() -> S + 'static,
1360 | |     S: Into<Oco<'static, str>>,
     | |_______________________________- first implementation here
...
1368 | / impl<T> From<Signal<T>> for TextProp
1369 | | where
1370 | |     T: Into<Self> + Clone,
     | |__________________________^ conflicting implementation for `TextProp`

This is because, as you predicted, these implementations create a conflict with the blanket implementation for Fn().

Fn() is only implemented for these types when the nightly feature is enabled, so you can add #[cfg(not(feature = "nightly))] to your additional implementations to avoid the conflict.

@SleeplessOne1917 SleeplessOne1917 deleted the signal-into-textprop branch February 5, 2024 05:53
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