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

Using html[_nested]! { ... } as a prop value produces an obscure error #2267

Closed
3 tasks
kawadakk opened this issue Dec 13, 2021 · 7 comments · Fixed by #2687
Closed
3 tasks

Using html[_nested]! { ... } as a prop value produces an obscure error #2267

kawadakk opened this issue Dec 13, 2021 · 7 comments · Fixed by #2687
Labels
A-yew-macro Area: The yew-macro crate bug

Comments

@kawadakk
Copy link
Contributor

kawadakk commented Dec 13, 2021

Problem
Using html! { ... } or html_nested! { ... } as a prop value in html! produces an obscure error.

Steps To Reproduce

  1. Write an html! { ... } expression in which there's another html_nested! { ... } directly inside a prop value. An example based on the pre-Add testing for website code blocks #2014 documentation:
use yew::{html, html_nested, virtual_dom::VChild, Component, Context, Html, Properties};

pub struct PageSideBar;

impl Component for PageSideBar {
    type Message = ();
    type Properties = ();

    fn create(_ctx: &Context<Self>) -> Self {
        Self
    }

    fn view(&self, _ctx: &Context<Self>) -> Html {
        html! {
            { "sidebar" }
        }
    }
}

#[derive(Properties, PartialEq)]
pub struct PageProps {
    #[prop_or_default]
    pub sidebar: Option<VChild<PageSideBar>>,
}

struct Page;

impl Component for Page {
    type Message = ();
    type Properties = PageProps;

    fn create(_ctx: &Context<Self>) -> Self {
        Self
    }

    fn view(&self, ctx: &Context<Self>) -> Html {
        html! {
            <div class="page">
                { ctx.props().sidebar.clone().map(Html::from).unwrap_or_default() }
                // ... page content
            </div>
        }
    }
}

// The page component can be called either with the sidebar or without:

pub fn render_page(with_sidebar: bool) -> Html {
    if with_sidebar {
        // Page with sidebar
        html! {
            <Page sidebar={html_nested! {  // <------------------------------------- HERE
                <PageSideBar />
            }} />
        }
    } else {
        // Page without sidebar
        html! {
            <Page />
        }
    }
}
  1. Observe the cryptic compilation error, wondering why html! doesn't treat html_nested! { ... } as an expression:
error: only an expression may be assigned as a property
  --> foo.rs:52:28
   |
52 |               <Page sidebar={html_nested! {
   |  ____________________________^
53 | |                 <PageSideBar />
54 | |             }} />
   | |_____________^

Expected behavior
Successful compilation or a helpful error messsage

Environment:

  • Yew version: v0.19

Questionnaire

  • I'm interested in fixing this myself but don't know where to start
  • I would like to fix and I have a solution
  • I don't have time to fix this right now, but maybe later
@kawadakk kawadakk added the bug label Dec 13, 2021
@ranile ranile added the A-yew-macro Area: The yew-macro crate label Dec 13, 2021
@ranile
Copy link
Member

ranile commented Dec 13, 2021

If html_nested! { ... } is wrapped in a set of { }, it works.

html! {
    <Page sidebar={{
        html_nested! { 
            <PageSideBar />
        }
    }} />
}

I have no idea what's happening here. This needs further investigation

@Madoshakalaka
Copy link
Contributor

will try to fix this

@Madoshakalaka
Copy link
Contributor

@kawadakk you followed outdated documentation, what you referred to was old behavior before and including 0.18.

here is the relevant documentation for 0.19 and it does expect extra braces same as the next/master version.

@hamza1311 the behavior change isn't introduced by me, but by dates before #2014. And it seems indeed unnatural to add these many pairs of braces though.

Do you think this issue can be closed?

@kawadakk
Copy link
Contributor Author

kawadakk commented Dec 16, 2021

The shown example was taken from a commit after #1939 (where the brace requirement for v0.19 was introduced) and before #2014 (documentation testing was introduced). #2014 added an extra pair of braces to make the example code compile.

Afaik it's never made clear which part was at fault in this change of #2014. I think one of the following things describes what happened:

  1. One possibility is that the example code really had an insufficient number of braces, and the macro was in the right to reject it. (I.e., prop={{html_nested! { ... }}} is correct)
  2. The other possibility I suspect that might be the case is that it was never intended to require an extra pair of braces for a prop value like html_nested! { ... }, and the macro inadvertently rejected the correct example code, but the PR author changed the example code anyway to make it compile. (I.e., prop={html_nested! { ... }} is correct)

In any case, the error message is still incorrect because html_nested! { ... } looks very like an expression.

@ranile
Copy link
Member

ranile commented Dec 16, 2021

To me, prop={html_nested! {}} is the correct one, especially since the following is correct:

let foo = html_nested! { ... };
html! {
    <Something prop={foo} />
}

@Madoshakalaka
Copy link
Contributor

You guys are right, will investigate and see what I can do then

@Madoshakalaka
Copy link
Contributor

I found the culprit, when parsing props of the form label=name, yew-macro expects the right-hand side to be an expression. And a macro call is not an expression
https://github.com/yewstack/yew/blob/master/packages/yew-macro/src/props/prop.rs#L113-L134

image

I had the creative idea of "eager expanding" to peek if the passed in macro will expand into an expression but that seems not possible in Rust after I've discussed the idea with other people.

I think considering this edge case and making the prop parser accept either Expr or Macro is the right way to go. Does anybody have other suggestions?

WorldSEnder added a commit to WorldSEnder/yew that referenced this issue May 16, 2022
convert macro invocation Item statements to Exprs
WorldSEnder added a commit that referenced this issue May 18, 2022
* fix: macro invocations in props #2267

convert macro invocation Item statements to Exprs

* remove now useless braces in docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-yew-macro Area: The yew-macro crate bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants