-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add shorthand syntax for props #1970
Conversation
Visit the preview URL for this PR (updated for commit 5cec835): https://yew-rs--pr1970-shortened-prop-synta-so64qc2v.web.app (expires Fri, 06 Aug 2021 16:38:28 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty cool change 👍 funny this is referenced as similar to svelte by others too and yet my first thought was this aligned well because Rust does this with structs:
struct Example {
value: &'static str,
}
// ..
let value = "Hello, world!";
let my_example = Example {
value
};
Specifying value: value
will make clippy unhappy.
So I think this syntax should not feel too foreign for rustaceans either 🦀.
I haven't looked through the documentation or examples fully to remove the old syntax because I don't think it's a bad thing for users to choose the explicit style so I'm happy if some sneak through the net, might even argue that we shouldn't remove all instances of the explicit style.
Apart from my comments below, I think we should add specific failure cases that cause the new errors for trybuild. I'm happy the errors work correctly but still think we should add it to trybuild tests for the future.
Co-authored-by: mc1098 <[email protected]>
…ed-prop-syntax
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy with the changes now the tests have been added to check the new errors :)
To reiterate the point I made earlier, I haven't checked if every instance the shorthand could be used; it is used as I think it's acceptable to use the more explicit style too.
@siku2 (sorry for the ping 👀) if you'd prefer the shorthand is always used now for examples/docs then I'm happy to review this PR again with that in mind.
I think it's fine for users to choose which syntax to use, so I don't think we should enforce usage of this shorthand, but I used a regex search to match all instances of the explicit syntax in order to change it, so I don't believe there are instances of it left in the documentation. |
I feel like it should emit a warning much like |
I have created an issue to address this later #1979. Is there anything else that needs to be done in this PR? |
Great, thanks!
Nope! |
Description
Now that braces are required around props, we can allow the svelte-style shorthand syntax for props:
Instead of
my_prop={my_prop}
, this allows you to just write{my_prop}
.Mentioned in #1769
Checklist
cargo make pr-flow