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

Normalize whitespaces in attribute values during compilation #6004

Closed
aradalvand opened this issue Feb 19, 2021 · 9 comments · Fixed by #6613
Closed

Normalize whitespaces in attribute values during compilation #6004

aradalvand opened this issue Feb 19, 2021 · 9 comments · Fixed by #6613

Comments

@aradalvand
Copy link

aradalvand commented Feb 19, 2021

Is your feature request related to a problem? Please describe.
It's not unusual to want to format your code (in this example particularly your CSS classes) in the following style:

image

In fact, I think Prettier and some other formatting tools encourage a similar thing, namely breaking a long line of code into several shorter ones, and Prettier specifically does it with CSS classes.

But this apparently causes the Svelte compiler to generate code like this:

class: button_class_value = "button\r\n           button--size--" + /*size*/ ctx[0] + "\r\n           button--theme--" + /*theme*/ ctx[1] + "\r\n           " + (/*$$restProps*/ ctx[6].class || "")

image

Notice all the unnecessary "\r"s, "\n"s, and multiple whitespace characters.
These:

  • Make the bundle size bigger completely unnecessarily.
  • Make the HTML output rather ugly:

image
Screenshot from Chrome DevTools

Describe the solution you'd like
Since multiple whitespaces, tabs, and line breaks inside HTML attribute values are always ignored by browsers and cause no functional difference, the Svelte compiler could easily ignore these characters, and "normalize" the whitespaces in attribute values, without causing any harm.
So that the result would look like this:

class: button_class_value = "button button--size--" + /*size*/ ctx[0] + " button--theme--" + /*theme*/ ctx[1] + (/*$$restProps*/ ctx[6].class || "")
@Conduitry
Copy link
Member

As of version 1.3.0, via sveltejs/prettier-plugin-svelte#145, the Prettier plugin no longer formats class attributes, to keep in line with Prettier's HTML formatting.

I have mixed feelings about whether the compiler should collapse whitespace in class attributes. It should definitely not collapse whitespace in all attributes, though.

@aradalvand
Copy link
Author

aradalvand commented Feb 20, 2021

@Conduitry Okay, but even if Prettier doesn't do that anymore, it still makes perfect sense to format your classes that way if you have a lot of them; otherwise if they're all on a single line it would become ridiculously long and pretty hard to look at! Wouldn't you agree?

I have mixed feelings about whether the compiler should collapse whitespace in class attributes.

I can't think of any cases where that could be undesirable or problematic, since it would make no functional difference whatsoever. (in the case of the class attribute at least, I'm not quite sure about others.)
Can you provide some examples? Why exactly do you have mixed feelings about this?

@kran6a
Copy link

kran6a commented Feb 20, 2021

It also happens to HTML only components even if you don't split the opening tag. You get a lot of \n and whitespaces that are not needed and increase the bundle size. Just commenting because I think a whitespace stripping option should also include these so we can create even smaller production bundles.
image

@aahmadi458
Copy link

aahmadi458 commented Feb 20, 2021

I agree. I think not only the classes but all the whitespaces in the entire HTML should be taken car eof, any redundant whitespaces should be removed. There is no reason not to do this.

As mentioned, they just increase the bundle size completely unnecessarily.

...It should definitely not collapse whitespace in all attributes, though.

Why shouldn't it?! In what attribute does that make any difference?

@dummdidumm
Copy link
Member

Removing whitespace inside HTML content (not attributes) can be dangerous because people might style the element such that it should preserve whitespace.

@aradalvand
Copy link
Author

aradalvand commented Feb 21, 2021

@dummdidumm Sure, I don't think Svelte should collapse whitespaces inside tags, although it'd be nice if we could have an option for that too, such that the developer could make the decision themself, because after all, most of the multiwhitespaces are just there for formatting, and so they ultimately add to the bundle size totally unnecessarily).

But in the case of attribute values, again, I can't think of any cases where multiple whitespaces would make any difference at all, and therefore there is no reason to preserve them there.

@stale
Copy link

stale bot commented Jun 26, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added stale-bot and removed stale-bot labels Jun 26, 2021
@aradalvand
Copy link
Author

aradalvand commented Jun 26, 2021

Hi there again.
Are there any reasons that make this idea non-feasible?

@Conduitry
Copy link
Member

In 3.42.2, class and style attributes now have their whitespace collapsed.

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

Successfully merging a pull request may close this issue.

6 participants