liveview-native / live_view_native

A framework for building native applications with Phoenix LiveView
https://native.live/
MIT License
592 stars 23 forks source link

Warn when the `style` attribute is missing a semicolon `;` #182

Open carson-katri opened 4 months ago

carson-katri commented 4 months ago

A common mistake is writing a style attribute with spaces as delimiters instead of semicolons ;:

<Element style="padding() background(.red)" />

This should log a warning, ideally pointing to the position where the semicolon should be inserted. A message similar to this could be used:

Missing semicolon between modifiers in `style` attribute:

<Element style="padding() background(.red)" />
                         ^ add a `;` semicolon after `padding()`

home_live.swiftui.neex:1:25

Alternatively, spaces could be allowed as a delimiter as well or instead of semicolons.

bcardarella commented 2 months ago

I just compared to web and in Chrome I don't see any support for whitespace deliminator between style rules so I'd like to not support that in ours.

But I agree some warning would be nice. @NduatiK I think we'd need to support that in the parser itself as currently I'm just doing String.split(style, ";")

NduatiK commented 2 months ago

Let's clarify:

@bcardarella, did I get that right?

bcardarella commented 2 months ago

so currently I split the value of the style attr on ; and just iterate through each rule one by one and send it to the parser. What I think we should do instead is send the entire string to the parser. The current RULES parser is splitting on newline, but we should probably introduce the optional ; to deliminate between rules:

padding(); background(.red)

should be the same as:

padding(); background(.red);

and

padding()
background(.red)

and

padding();
background(.red);

Does that make sense?

NduatiK commented 2 months ago

Makes sense. One more clarification, would a missing semicolon be expected on sheets and produce a warning?

bcardarella commented 2 months ago

are you asking if the following:

padding();
fooBar()
barBaz();

where the 2nd rule has no semicolor?

NduatiK commented 2 months ago

Yes, should the SHEET sigil produce a warning if the 2nd rule's semicolon is missing?

I'd like to know if warnings should always be produced or if it should be configurable.

bcardarella commented 2 months ago

That's a good question, are semicolons required for multiline in css or not? I forget what the current state of that is. If it is then we should probably just follow that but it may need to be a v0.4 thing and accept the breaking change

cmnstmntmn commented 1 month ago

Going to link this issue https://github.com/liveview-native/liveview-client-swiftui/issues/1421, since they might use the same parser internally.