Skip to content

Commit

Permalink
Evaluate props in the order they're defined (#2887)
Browse files Browse the repository at this point in the history
* don't change order of props

* rename `SortedPropList` to `PropList`

* docs + test

* remove dead code

* fmt
  • Loading branch information
hamza1311 committed Oct 8, 2022
1 parent 32b3150 commit 426a1fd
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 38 deletions.
44 changes: 14 additions & 30 deletions packages/yew-macro/src/props/prop.rs
@@ -1,4 +1,3 @@
use std::cmp::Ordering;
use std::convert::TryFrom;
use std::ops::{Deref, DerefMut};

Expand All @@ -9,7 +8,6 @@ use syn::spanned::Spanned;
use syn::token::Brace;
use syn::{braced, Block, Expr, ExprBlock, ExprPath, ExprRange, Stmt, Token};

use super::CHILDREN_LABEL;
use crate::html_tree::HtmlDashedName;
use crate::stringify::Stringify;

Expand All @@ -24,6 +22,7 @@ pub struct Prop {
/// Punctuation between `label` and `value`.
pub value: Expr,
}

impl Parse for Prop {
fn parse(input: ParseStream) -> syn::Result<Self> {
let directive = input
Expand Down Expand Up @@ -216,36 +215,21 @@ fn advance_until_next_dot2(input: &ParseBuffer) -> syn::Result<()> {
///
/// The list may contain multiple props with the same label.
/// Use `check_no_duplicates` to ensure that there are no duplicates.
pub struct SortedPropList(Vec<Prop>);
impl SortedPropList {
pub struct PropList(Vec<Prop>);
impl PropList {
/// Create a new `SortedPropList` from a vector of props.
/// The given `props` doesn't need to be sorted.
pub fn new(mut props: Vec<Prop>) -> Self {
props.sort_by(|a, b| Self::cmp_label(&a.label.to_string(), &b.label.to_string()));
pub fn new(props: Vec<Prop>) -> Self {
Self(props)
}

fn cmp_label(a: &str, b: &str) -> Ordering {
if a == b {
Ordering::Equal
} else if a == CHILDREN_LABEL {
Ordering::Greater
} else if b == CHILDREN_LABEL {
Ordering::Less
} else {
a.cmp(b)
}
}

fn position(&self, key: &str) -> Option<usize> {
self.0
.binary_search_by(|prop| Self::cmp_label(prop.label.to_string().as_str(), key))
.ok()
self.0.iter().position(|it| it.label.to_string() == key)
}

/// Get the first prop with the given key.
pub fn get_by_label(&self, key: &str) -> Option<&Prop> {
self.position(key).and_then(|i| self.0.get(i))
self.0.iter().find(|it| it.label.to_string() == key)
}

/// Pop the first prop with the given key.
Expand Down Expand Up @@ -312,7 +296,7 @@ impl SortedPropList {
}))
}
}
impl Parse for SortedPropList {
impl Parse for PropList {
fn parse(input: ParseStream) -> syn::Result<Self> {
let mut props: Vec<Prop> = Vec::new();
// Stop parsing props if a base expression preceded by `..` is reached
Expand All @@ -323,7 +307,7 @@ impl Parse for SortedPropList {
Ok(Self::new(props))
}
}
impl Deref for SortedPropList {
impl Deref for PropList {
type Target = [Prop];

fn deref(&self) -> &Self::Target {
Expand All @@ -340,7 +324,7 @@ impl SpecialProps {
const KEY_LABEL: &'static str = "key";
const REF_LABEL: &'static str = "ref";

fn pop_from(props: &mut SortedPropList) -> syn::Result<Self> {
fn pop_from(props: &mut PropList) -> syn::Result<Self> {
let node_ref = props.pop_unique(Self::REF_LABEL)?;
let key = props.pop_unique(Self::KEY_LABEL)?;
Ok(Self { node_ref, key })
Expand Down Expand Up @@ -386,15 +370,15 @@ impl SpecialProps {

pub struct Props {
pub special: SpecialProps,
pub prop_list: SortedPropList,
pub prop_list: PropList,
}
impl Parse for Props {
fn parse(input: ParseStream) -> syn::Result<Self> {
Self::try_from(input.parse::<SortedPropList>()?)
Self::try_from(input.parse::<PropList>()?)
}
}
impl Deref for Props {
type Target = SortedPropList;
type Target = PropList;

fn deref(&self) -> &Self::Target {
&self.prop_list
Expand All @@ -406,10 +390,10 @@ impl DerefMut for Props {
}
}

impl TryFrom<SortedPropList> for Props {
impl TryFrom<PropList> for Props {
type Error = syn::Error;

fn try_from(mut prop_list: SortedPropList) -> Result<Self, Self::Error> {
fn try_from(mut prop_list: PropList) -> Result<Self, Self::Error> {
let special = SpecialProps::pop_from(&mut prop_list)?;
Ok(Self { special, prop_list })
}
Expand Down
4 changes: 2 additions & 2 deletions packages/yew-macro/src/props/prop_macro.rs
Expand Up @@ -8,7 +8,7 @@ use syn::spanned::Spanned;
use syn::token::Brace;
use syn::{Expr, Token, TypePath};

use super::{ComponentProps, Prop, Props, SortedPropList};
use super::{ComponentProps, Prop, PropList, Props};
use crate::html_tree::HtmlDashedName;

/// Pop from `Punctuated` without leaving it in a state where it has trailing punctuation.
Expand Down Expand Up @@ -106,7 +106,7 @@ pub struct PropsMacroInput {
impl Parse for PropsMacroInput {
fn parse(input: ParseStream) -> syn::Result<Self> {
let PropsExpr { ty, fields, .. } = input.parse()?;
let prop_list = SortedPropList::new(fields.into_iter().map(Into::into).collect());
let prop_list = PropList::new(fields.into_iter().map(Into::into).collect());
let props: Props = prop_list.try_into()?;
props.special.check_all(|prop| {
let label = &prop.label;
Expand Down
4 changes: 2 additions & 2 deletions packages/yew-macro/tests/html_macro/component-fail.stderr
Expand Up @@ -143,10 +143,10 @@ error: `with` doesn't have a value. (hint: set the value to `true` or `false` fo
| ^^^^

error: `ref` can only be specified once
--> tests/html_macro/component-fail.rs:67:20
--> tests/html_macro/component-fail.rs:67:29
|
67 | html! { <Child ref={()} ref={()} value=1 ..props /> };
| ^^^
| ^^^

error: `with` doesn't have a value. (hint: set the value to `true` or `false` for boolean attributes)
--> tests/html_macro/component-fail.rs:68:20
Expand Down
8 changes: 4 additions & 4 deletions packages/yew-macro/tests/html_macro/element-fail.stderr
Expand Up @@ -101,16 +101,16 @@ error: `class` can only be specified once but is given here again
| ^^^^^

error: `ref` can only be specified once
--> tests/html_macro/element-fail.rs:33:20
--> tests/html_macro/element-fail.rs:33:29
|
33 | html! { <input ref={()} ref={()} /> };
| ^^^
| ^^^

error: `ref` can only be specified once
--> tests/html_macro/element-fail.rs:63:20
--> tests/html_macro/element-fail.rs:63:29
|
63 | html! { <input ref={()} ref={()} /> };
| ^^^
| ^^^

error: the tag `<input>` is a void element and cannot have children (hint: rewrite this as `<input/>`)
--> tests/html_macro/element-fail.rs:66:13
Expand Down
21 changes: 21 additions & 0 deletions packages/yew-macro/tests/props_macro_test.rs
Expand Up @@ -5,3 +5,24 @@ fn props_macro() {
t.pass("tests/props_macro/*-pass.rs");
t.compile_fail("tests/props_macro/*-fail.rs");
}

#[test]
fn props_order() {
#[derive(yew::Properties, PartialEq)]
struct Props {
first: usize,
second: usize,
last: usize,
}

let mut g = 1..=3;
let props = yew::props!(Props {
first: g.next().unwrap(),
second: g.next().unwrap(),
last: g.next().unwrap()
});

assert_eq!(props.first, 1);
assert_eq!(props.second, 2);
assert_eq!(props.last, 3);
}
18 changes: 18 additions & 0 deletions website/docs/concepts/function-components/properties.mdx
Expand Up @@ -263,6 +263,24 @@ fn App() -> Html {
}
```

## Evaluation Order

Props are evaluated in the order they're specified, as shown by the following example:

```rust
#[derive(yew::Properties, PartialEq)]
struct Props { first: usize, second: usize, last: usize }

fn main() {
let mut g = 1..=3;
let props = yew::props!(Props { first: g.next().unwrap(), second: g.next().unwrap(), last: g.next().unwrap() });

assert_eq!(props.first, 1);
assert_eq!(props.second, 2);
assert_eq!(props.last, 3);
}
```

## Anti Patterns

While almost any Rust type can be passed as properties, there are some anti-patterns that should be avoided.
Expand Down

0 comments on commit 426a1fd

Please sign in to comment.