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: add impl_from argument to #[server] proc_macro #2335

Merged
merged 6 commits into from
Feb 20, 2024

Conversation

videobitva
Copy link
Contributor

@videobitva videobitva commented Feb 17, 2024

This PR adds an optional impl_from named argument for server_fn proc macro.
This argument defaults to true and enables (or disables) the impl of From trait for server function argument type.

In cases when the server function has more than one argument, the impl_from has no effect on the trait impl generation, as it has been already disabled by function arguments number check.

The related issue has been discussed here: https://discord.com/channels/1031524867910148188/1203001939089035295

TL;DR: orphan rules disable (in some cases) the implementation of From trait for conversion between the only server function argument and server function argument type.

The minimal example of the issue lives here: https://github.com/videobitva/test_server_fn

Copy link
Collaborator

@gbj gbj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good overall. See comments above about naming and documentation.

leptos_macro/src/lib.rs Outdated Show resolved Hide resolved
server_fn_macro/src/lib.rs Outdated Show resolved Hide resolved
Rename `impl_into` back to be `impl_from`
Rework docstring
@videobitva videobitva changed the title feat: add impl_into argument to #[server] proc_macro feat: add impl_from argument to #[server] proc_macro Feb 19, 2024
@gbj
Copy link
Collaborator

gbj commented Feb 20, 2024

Excellent, thanks!

@gbj gbj merged commit aa3700f into leptos-rs:main Feb 20, 2024
60 checks passed
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