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

Add "struct update" syntax to pass props to component (..props instead of with props) #2024

Merged
merged 24 commits into from
Sep 6, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
9759c39
Reword to use double-dot syntax instead of "with"
Xavientois Aug 29, 2021
ca7479b
Implement double-dot syntax for props in components
Xavientois Aug 30, 2021
2b53e00
Update documentation with new syntax
Xavientois Aug 30, 2021
3bc1def
Update forgotten doc
Xavientois Aug 30, 2021
102ccaf
Merge branch 'master' into add-double-dot-prop-syntax
Xavientois Aug 30, 2021
afc22d0
Add descriptive comments
Xavientois Aug 30, 2021
de0c69a
Merge branch 'add-double-dot-prop-syntax' of github.com:Xavientois/ye…
Xavientois Aug 30, 2021
db2598f
Check props and base expression
Xavientois Aug 30, 2021
503ef88
Make compatible with 1.49.0 by removing then
Xavientois Aug 30, 2021
aa94eff
Fix website tests
Xavientois Aug 30, 2021
04c90de
Update error output
Xavientois Aug 30, 2021
e2ef587
Implicitly convert string literals to String if they are listed as props
Xavientois Aug 30, 2021
6f9ea0f
Remove unused keyword
Xavientois Aug 30, 2021
920daf3
Rename function for checking if string literal
Xavientois Aug 30, 2021
dc43274
Fix weird formatting
Xavientois Aug 30, 2021
b7a312b
Update code based on review
Xavientois Aug 31, 2021
e78cb2a
Update website/docs/concepts/html/components.md
Xavientois Aug 31, 2021
0922b19
Base expression span includes dot2 now
Xavientois Sep 1, 2021
aba0cdc
Improve specificity of error message
Xavientois Sep 1, 2021
7ab9719
Chain together error messages
Xavientois Sep 1, 2021
e79ea1d
Add an example failure case to illustrate combined error message
Xavientois Sep 1, 2021
b75e16a
Update based on review comments
Xavientois Sep 4, 2021
d68826e
Merge branch 'master' into add-double-dot-prop-syntax
Xavientois Sep 5, 2021
79599d9
Fix missing clones
Xavientois Sep 6, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions packages/yew-macro/src/props/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ use syn::{
Expr, ExprLit, Lit,
};

mod kw {
syn::custom_keyword!(with);
}

pub struct ComponentProps {
props: Props,
base_expr: Option<Expr>,
Expand Down Expand Up @@ -91,7 +87,8 @@ impl ComponentProps {
Some(expr) => {
let ident = Ident::new("__yew_props", props_ty.span());
let set_props = self.props.iter().map(|Prop { label, value, .. }| {
if is_implicitly_converted(value) {
if is_string_literal(value) {
// String literals should be implicitly converted into `String`
quote_spanned! {value.span()=>
#ident.#label = #value.into();
Xavientois marked this conversation as resolved.
Show resolved Hide resolved
}
Expand Down Expand Up @@ -125,7 +122,7 @@ impl ComponentProps {
}
}

fn is_implicitly_converted(expr: &Expr) -> bool {
fn is_string_literal(expr: &Expr) -> bool {
matches!(
expr,
Expr::Lit(ExprLit {
Expand All @@ -148,7 +145,10 @@ impl Parse for ComponentProps {
if input.is_empty() {
Ok(Self { props, base_expr })
} else {
Err(input.error("base props expression must appear last in list of props"))
Err(syn::Error::new_spanned(
base_expr,
"base props expression must appear last in list of props",
))
}
}
}
Expand Down
35 changes: 19 additions & 16 deletions packages/yew-macro/src/props/prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,25 +85,28 @@ impl Prop {
fn parse_prop_value(input: &ParseBuffer) -> syn::Result<Expr> {
if input.peek(Brace) {
strip_braces(input.parse()?)
} else if let Some(ExprRange {
from: Some(from), ..
}) = range_expression_peek(input)
{
// If a range expression is seen, treat the left-side expression as the value
// and leave the right-side expression to be parsed as a base expression
advance_until_next_dot2(input)?;
Ok(*from)
} else {
let expr = input.parse()?;
let expr = if let Some(ExprRange {
from: Some(from), ..
}) = range_expression_peek(input)
{
// If a range expression is seen, treat the left-side expression as the value
// and leave the right-side expression to be parsed as a base expression
advance_until_next_dot2(input)?;
*from
} else {
input.parse()?
};

match &expr {
Expr::Lit(_) => Ok(expr),
_ => {
Err(syn::Error::new_spanned(
&expr,
"the property value must be either a literal or enclosed in braces. Consider adding braces around your expression.".to_string(),
))
}
Expr::Lit(_) => Ok(expr),
_ => {
Err(syn::Error::new_spanned(
&expr,
"the property value must be either a literal or enclosed in braces. Consider adding braces around your expression.".to_string(),
Xavientois marked this conversation as resolved.
Show resolved Hide resolved
))
}
}
}
}

Expand Down
3 changes: 3 additions & 0 deletions packages/yew-macro/tests/html_macro/component-fail.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ fn compile_fail() {
html! { <Child></Child><Child></Child> };
html! { <Child>{ "Not allowed" }</Child> };

let num = 1;
html! { <Child int=num ..props /> };

// trying to overwrite `children` on props which don't take any.
html! {
<Child ..ChildProperties { string: "hello".to_owned(), int: 5 }>
Expand Down
108 changes: 57 additions & 51 deletions packages/yew-macro/tests/html_macro/component-fail.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ error: `with` doesn't have a value. (hint: set the value to `true` or `false` fo
| ^^^^

error: base props expression must appear last in list of props
--> $DIR/component-fail.rs:54:28
--> $DIR/component-fail.rs:54:22
|
54 | html! { <Child ..props ref={()} ref={()} /> };
| ^^^
| ^^^^^
Xavientois marked this conversation as resolved.
Show resolved Hide resolved

error: `with` doesn't have a value. (hint: set the value to `true` or `false` for boolean attributes)
--> $DIR/component-fail.rs:55:20
Expand All @@ -65,10 +65,10 @@ error: `with` doesn't have a value. (hint: set the value to `true` or `false` fo
| ^^^^

error: base props expression must appear last in list of props
--> $DIR/component-fail.rs:56:28
--> $DIR/component-fail.rs:56:22
|
56 | html! { <Child ..props ref={()} ref={()} value=1 /> };
| ^^^
| ^^^^^

error: `with` doesn't have a value. (hint: set the value to `true` or `false` for boolean attributes)
--> $DIR/component-fail.rs:57:20
Expand All @@ -77,10 +77,10 @@ error: `with` doesn't have a value. (hint: set the value to `true` or `false` fo
| ^^^^

error: base props expression must appear last in list of props
--> $DIR/component-fail.rs:58:28
--> $DIR/component-fail.rs:58:22
|
58 | html! { <Child ..props ref={()} value=1 ref={()} /> };
| ^^^
| ^^^^^

error: `with` doesn't have a value. (hint: set the value to `true` or `false` for boolean attributes)
--> $DIR/component-fail.rs:59:20
Expand All @@ -89,10 +89,10 @@ error: `with` doesn't have a value. (hint: set the value to `true` or `false` fo
| ^^^^

error: base props expression must appear last in list of props
--> $DIR/component-fail.rs:60:28
--> $DIR/component-fail.rs:60:22
|
60 | html! { <Child ..props value=1 ref={()} ref={()} /> };
| ^^^^^
| ^^^^^

error: `with` doesn't have a value. (hint: set the value to `true` or `false` for boolean attributes)
--> $DIR/component-fail.rs:61:28
Expand All @@ -101,10 +101,10 @@ error: `with` doesn't have a value. (hint: set the value to `true` or `false` fo
| ^^^^

error: base props expression must appear last in list of props
--> $DIR/component-fail.rs:62:37
--> $DIR/component-fail.rs:62:30
|
62 | html! { <Child value=1 ..props ref={()} ref={()} /> };
| ^^^
| ^^^^^

error: `with` doesn't have a value. (hint: set the value to `true` or `false` for boolean attributes)
--> $DIR/component-fail.rs:63:37
Expand All @@ -113,10 +113,10 @@ error: `with` doesn't have a value. (hint: set the value to `true` or `false` fo
| ^^^^

error: base props expression must appear last in list of props
--> $DIR/component-fail.rs:64:45
--> $DIR/component-fail.rs:64:39
|
64 | html! { <Child value=1 ref={()} ..props ref={()} /> };
| ^^^
| ^^^^^

error: `with` doesn't have a value. (hint: set the value to `true` or `false` for boolean attributes)
--> $DIR/component-fail.rs:65:47
Expand All @@ -131,10 +131,10 @@ error: `ref` can only be specified once
| ^^^

error: base props expression must appear last in list of props
--> $DIR/component-fail.rs:69:29
--> $DIR/component-fail.rs:69:23
|
69 | html! { <Child .. props value=1 /> };
| ^^^^^
| ^^^^^

error: expected identifier, found keyword `type`
--> $DIR/component-fail.rs:70:20
Expand Down Expand Up @@ -201,34 +201,40 @@ error: only one root html element is allowed (hint: you can wrap multiple html e
85 | html! { <Child></Child><Child></Child> };
| ^^^^^^^^^^^^^^^

error: the property value must be either a literal or enclosed in braces. Consider adding braces around your expression.
--> $DIR/component-fail.rs:89:24
|
89 | html! { <Child int=num ..props /> };
| ^^^

error: cannot specify the `children` prop when the component already has children
--> $DIR/component-fail.rs:104:26
--> $DIR/component-fail.rs:107:26
|
104 | <ChildContainer {children}>
107 | <ChildContainer {children}>
| ^^^^^^^^

error: only one root html element is allowed (hint: you can wrap multiple html elements in a fragment `<></>`)
--> $DIR/component-fail.rs:111:9
--> $DIR/component-fail.rs:114:9
|
111 | <span>{ 2 }</span>
114 | <span>{ 2 }</span>
| ^^^^^^^^^^^^^^^^^^

error: only simple identifiers are allowed in the shorthand property syntax
--> $DIR/component-fail.rs:114:21
--> $DIR/component-fail.rs:117:21
|
114 | html! { <Child {std::f64::consts::PI} /> };
117 | html! { <Child {std::f64::consts::PI} /> };
| ^^^^^^^^^^^^^^^^^^^^

error: missing label for property value. If trying to use the shorthand property syntax, only identifiers may be used
--> $DIR/component-fail.rs:115:21
--> $DIR/component-fail.rs:118:21
|
115 | html! { <Child {7 + 6} /> };
118 | html! { <Child {7 + 6} /> };
| ^^^^^

error: missing label for property value. If trying to use the shorthand property syntax, only identifiers may be used
--> $DIR/component-fail.rs:116:21
--> $DIR/component-fail.rs:119:21
|
116 | html! { <Child {children.len()} /> };
119 | html! { <Child {children.len()} /> };
| ^^^^^^^^^^^^^^

error[E0425]: cannot find value `blah` in this scope
Expand Down Expand Up @@ -376,54 +382,54 @@ error[E0599]: no method named `children` found for struct `ChildPropertiesBuilde
| ^^^^^ method not found in `ChildPropertiesBuilder<ChildPropertiesBuilderStep_missing_required_prop_int>`

error[E0609]: no field `children` on type `ChildProperties`
--> $DIR/component-fail.rs:90:10
--> $DIR/component-fail.rs:93:10
|
90 | <Child ..ChildProperties { string: "hello".to_owned(), int: 5 }>
93 | <Child ..ChildProperties { string: "hello".to_owned(), int: 5 }>
| ^^^^^ unknown field
|
= note: available fields are: `string`, `int`

error[E0599]: no method named `build` found for struct `ChildContainerPropertiesBuilder<ChildContainerPropertiesBuilderStep_missing_required_prop_children>` in the current scope
--> $DIR/component-fail.rs:95:14
--> $DIR/component-fail.rs:98:14
|
24 | #[derive(Clone, Properties, PartialEq)]
| ---------- method `build` not found for this
...
95 | html! { <ChildContainer /> };
98 | html! { <ChildContainer /> };
| ^^^^^^^^^^^^^^ method not found in `ChildContainerPropertiesBuilder<ChildContainerPropertiesBuilderStep_missing_required_prop_children>`

error[E0599]: no method named `build` found for struct `ChildContainerPropertiesBuilder<ChildContainerPropertiesBuilderStep_missing_required_prop_children>` in the current scope
--> $DIR/component-fail.rs:96:14
--> $DIR/component-fail.rs:99:14
|
24 | #[derive(Clone, Properties, PartialEq)]
| ---------- method `build` not found for this
...
96 | html! { <ChildContainer></ChildContainer> };
99 | html! { <ChildContainer></ChildContainer> };
| ^^^^^^^^^^^^^^ method not found in `ChildContainerPropertiesBuilder<ChildContainerPropertiesBuilderStep_missing_required_prop_children>`

error[E0277]: the trait bound `VChild<Child>: From<yew::virtual_dom::VText>` is not satisfied
--> $DIR/component-fail.rs:97:31
|
97 | html! { <ChildContainer>{ "Not allowed" }</ChildContainer> };
| ^^^^^^^^^^^^^ the trait `From<yew::virtual_dom::VText>` is not implemented for `VChild<Child>`
|
= note: required because of the requirements on the impl of `Into<VChild<Child>>` for `yew::virtual_dom::VText`
= note: required by `into`
--> $DIR/component-fail.rs:100:31
|
100 | html! { <ChildContainer>{ "Not allowed" }</ChildContainer> };
| ^^^^^^^^^^^^^ the trait `From<yew::virtual_dom::VText>` is not implemented for `VChild<Child>`
|
= note: required because of the requirements on the impl of `Into<VChild<Child>>` for `yew::virtual_dom::VText`
= note: required by `into`

error[E0277]: the trait bound `VChild<Child>: From<VNode>` is not satisfied
--> $DIR/component-fail.rs:98:29
|
98 | html! { <ChildContainer><></></ChildContainer> };
| ^ the trait `From<VNode>` is not implemented for `VChild<Child>`
|
= note: required because of the requirements on the impl of `Into<VChild<Child>>` for `VNode`
= note: required by `into`
--> $DIR/component-fail.rs:101:29
|
101 | html! { <ChildContainer><></></ChildContainer> };
| ^ the trait `From<VNode>` is not implemented for `VChild<Child>`
|
= note: required because of the requirements on the impl of `Into<VChild<Child>>` for `VNode`
= note: required by `into`

error[E0277]: the trait bound `VChild<Child>: From<VNode>` is not satisfied
--> $DIR/component-fail.rs:99:30
|
99 | html! { <ChildContainer><other /></ChildContainer> };
| ^^^^^ the trait `From<VNode>` is not implemented for `VChild<Child>`
|
= note: required because of the requirements on the impl of `Into<VChild<Child>>` for `VNode`
= note: required by `into`
--> $DIR/component-fail.rs:102:30
|
102 | html! { <ChildContainer><other /></ChildContainer> };
| ^^^^^ the trait `From<VNode>` is not implemented for `VChild<Child>`
|
= note: required because of the requirements on the impl of `Into<VChild<Child>>` for `VNode`
= note: required by `into`
7 changes: 6 additions & 1 deletion packages/yew-macro/tests/html_macro/component-pass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ fn compile_pass() {


let props = <Container as Component>::Properties::default();
let child_props = <Child as Component>::Properties::default();
html! {
<>
<Container int=1 />
Expand All @@ -197,10 +198,14 @@ fn compile_pass() {
<div>{ "hello world" }</div>
</Container>

<Container int=1 ..props>
<Container int=1 ..props.clone()>
<div>{ "hello world" }</div>
</Container>

<Container int=1 ..props>
<Child int=2 string="hello" ..child_props />
</Container>

<Container int=1>
<Child int=2 />
</Container>
Expand Down
5 changes: 4 additions & 1 deletion website/docs/concepts/html/components.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,10 @@ html! {
};
```

When using the struct update syntax (passing props as `..props`), the children passed in the `html!` macro overwrite the ones already present in the props.
The `html!` macro allows you to pass a base expression with the `..props` syntax instead of specifying each property individually,
similarly to Rust's [Functional Update Syntax](https://doc.rust-lang.org/stable/reference/expressions/struct-expr.html#functional-update-syntax).
Xavientois marked this conversation as resolved.
Show resolved Hide resolved
This base expression must occur after any individual props are passed.
When passing a base props expression with a `children` field, the children passed in the `html!` macro overwrite the ones already present in the props.

```rust
use yew::{Children, Component, Context, html, Html, props, Properties};
Expand Down