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

view! macro with the with! macro leads to weird formatting #97

Closed
DolceTriade opened this issue Dec 19, 2023 · 8 comments · Fixed by #121
Closed

view! macro with the with! macro leads to weird formatting #97

DolceTriade opened this issue Dec 19, 2023 · 8 comments · Fixed by #121

Comments

@DolceTriade
Copy link

DolceTriade commented Dec 19, 2023

Hello,

thanks for this great project!

I've noticed some inconsistent behavior with some code like this:

    #[test]
    fn with_with_macro() {
        let formatted = view_macro!(view! { {move||with!(|scope| view!{ <div>"hi"</div> })}});
        insta::assert_snapshot!(formatted, @r#"
        view! {
            {move || with!(|scope| view! { 
                <div>
                    "hi"
                </div>
            })
            }}
        }
        "#);
    }

I might expect this to be formatting something like in the test... However this is formatted like:

view! { {move || with!(| scope | view! { < div > "hi" < / div > })} }

It just adds spaces everywhere in the statement which actually makes the code quite unreadable for more complex examples.

This is the simplest case I found.

@bram209
Copy link
Owner

bram209 commented Dec 19, 2023

Hi,

Rust macros can take arbitrary streams of tokens. It does not have to be valid rust syntax, therefore this formatter cannot know how the contents of a macro need to be formatted.

@DolceTriade
Copy link
Author

DolceTriade commented Dec 19, 2023

Is there a way to make leptosfmt to treat certain macros, such as those provided by leptos as valid?

Happy to take a stab at providing a PR if you can point me in the right direction.

either that, or can we make leptosfmt not modify code inside macros instead of formatting it badly?

@bram209
Copy link
Owner

bram209 commented Dec 21, 2023

could you tell me more about your use case, what does with! do?

@DolceTriade
Copy link
Author

DolceTriade commented Dec 21, 2023

https://github.com/leptos-rs/leptos/releases/tag/v0.5.0-rc1

From the leptos release notes:
with!() and update!() macros
Nested .withI() calls are a pain. Now you can do

let (first, _) = create_signal("Bob".to_string());
let (middle, _) = create_signal("J.".to_string());
let (last, _) = create_signal("Smith".to_string());
let name = move || with!(|first, middle, last| format!("{first} {middle} {last}"));

instead of

let name = move || {
	first.with(|first| {
		middle.with(|middle| last.with(|last| format!("{first} {middle} {last}")))
	})
};

It is a a new macro leptos provides to simplify using multiple signals with the .with method. Basically syntactic sugar for nested signal.with calls.

@bram209
Copy link
Owner

bram209 commented Dec 21, 2023

Ah, I wasn't aware of with! with_value! and update! macros. I didn't know they were leptos specific.

Wouldn't it be more performant to do:

view! { <div>{with!(...)}</div>

instead of

with!(|...| view! { <div>....</div> }) as you would be recreating the dom element with every observed change?

Is it common to use call the view! macro inside these macros? If so, I could make leptosfmt aware of these as they are leptos specific.

It just adds spaces everywhere in the statement which actually makes the code quite unreadable for more complex examples.

I do agree that the default formatting behaviour of unknown macros is far from ideal. The least it should do is keep the original formatting intact.

@DolceTriade
Copy link
Author

DolceTriade commented Dec 21, 2023

Wouldn't it be more performant to do:

Probably, but not all code is optimized right away 🤷. The other approach is valid and functionally performant so it shouldn't be mangled at the least. As an interim, I have avoided using the with! macro which seems sufficient for now.

imo, without this tool, using leptos is really hard due to rustfmt not being able to format view! macros, so it would be nice to support this.

@stevepryde
Copy link

I've noticed leptosfmt tends to mess up the contents of most other macros other than view!. Shouldn't it just leave code in other macros alone? It seems far too invasive at the moment.

@bram209
Copy link
Owner

bram209 commented Feb 14, 2024

I've noticed leptosfmt tends to mess up the contents of most other macros other than view!. Shouldn't it just leave code in other macros alone? It seems far too invasive at the moment.

Indeed, however this is the default behaviour of the prettyplease crate and didn't got around pushing an update yet.

See my previous comment in this issue:

I do agree that the default formatting behaviour of unknown macros is far from ideal. The least it should do is keep the original formatting intact.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants