Skip to content

Conversation

@jjvn84
Copy link
Contributor

@jjvn84 jjvn84 commented Apr 23, 2025

This is an attempt at solving issue #3945 when parsing attributes.
I added code that checks if there is a TokenTree::Group followed by a dot. If it finds it, it parses the whole attribute as an expression. It stops if it finds a comma, signaling the end of the attribute.
I added the example in the original issue to the "complex_kitchen_sink" test, and everything passed.

@jjvn84 jjvn84 requested a review from a team as a code owner April 23, 2025 14:52
Copy link
Member

@ealmloff ealmloff left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! Unfortunately, I think the issue is slightly broader than just method calls. If we find anything except a comma after a braced expression we should try to parse it as a normal expression. The check you added fixes this expression width: {"100%"}.to_string(),, but this expression still fails width: {|| "100%"}(),

@ealmloff ealmloff added bug Something isn't working rsx Related to rsx or the dioxus-rsx crate labels Apr 28, 2025
@jjvn84
Copy link
Contributor Author

jjvn84 commented Apr 28, 2025

@ealmloff These new changes should fix the issue. Could you please check again if there is another case I'm missing?
And thank you for the review.

@jjvn84 jjvn84 requested a review from ealmloff April 28, 2025 18:50
@ealmloff
Copy link
Member

I found a few more cases that are broken. This test fails on the PR, but passes on the main branch:

#[test]
fn invalid_braced_expression() {
    let item = quote::quote! {
        // Partial expressions in braces rsx should be allowed and output as-is
        // for autocomplete
        {invalid.}
        div {}
        {invalid.}
        // Comments should be ignored
        div {}
        {invalid.}
        "hello world"
        {invalid.}
        if true {}
    };

    let cb: CallBody = syn::parse2(item).unwrap();
    println!("{}", cb.to_token_stream());
}

These all pass now which is great!

#[test]
fn expressions_after_braced() {
    let item = quote::quote! {
        div {
            width: {100} - 50,
            width: {"100%"}.to_string(),
            width: {|| "100%"}(),
        }
    };

    let cb: CallBody = syn::parse2(item).unwrap();
    println!("{}", cb.to_token_stream());
}

I think these two cases should cover everything?

  1. Braced expression followed by a symbol other than a brace (-, *, (, etc) should be parsed as a syn::Expr
  2. Braced expression followed by a term which is either another braced expression, a raw ident or a literal should be parsed as a partial expression

@jjvn84
Copy link
Contributor Author

jjvn84 commented May 1, 2025

This new change fixes the missing cases. Please let me know if there are more test cases I should include.

Copy link
Member

@ealmloff ealmloff left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for fixing this! Confirmed autocomplete still works with the base raw brace expressions

@ealmloff ealmloff merged commit 71d85e6 into DioxusLabs:main May 2, 2025
16 of 17 checks passed
AnteDeliria pushed a commit to AnteDeliria/dioxus that referenced this pull request Jun 2, 2025
* Fix parsing of braced expressions followed by a method

* Improves the logic for parsing braced expressions

* Fix hot-reload issue

* Added cases for braced expressions followed by identifiers or string literals
AnteDeliria pushed a commit to AnteDeliria/dioxus that referenced this pull request Jul 23, 2025
* Fix parsing of braced expressions followed by a method

* Improves the logic for parsing braced expressions

* Fix hot-reload issue

* Added cases for braced expressions followed by identifiers or string literals
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working rsx Related to rsx or the dioxus-rsx crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants