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

Document that compressHTML doesn't remove all whitespace #7502

Closed
1 task
mayank99 opened this issue Mar 20, 2024 · 16 comments
Closed
1 task

Document that compressHTML doesn't remove all whitespace #7502

mayank99 opened this issue Mar 20, 2024 · 16 comments
Labels
good first issue Good for newcomers help wanted Issues looking for someone to run with them! improve documentation Enhance existing documentation (e.g. add an example, improve description)

Comments

@mayank99
Copy link
Contributor

mayank99 commented Mar 20, 2024

Astro Info

Astro:  v4.5.6
Output: static

Describe the Bug

Newlines are left as real whitespace in the HTML output, instead of being removed.

Input

<p>
 A
 <span>B</span>
 C
</p>

results in this output:

<p>
A
<span>B</span>
C
</p>

which looks like "A B C" when rendered (instead of "ABC").

The documentation says:

By default, Astro removes all whitespace from your HTML, including line breaks, from .astro components. This occurs both in development mode and in the final build.

It looks like this behavior might have changed in withastro/astro#7556 / compiler#816?

What's the expected result?

All whitespace should be collapsed, like the documentation says.

e.g. this should be the output:

<p>A<span>B</span>C</p>

If real whitespace is desired, it can always be manually added using {" "}. This makes the behavior more predictable.


If the current behavior is intentional, then it should be documented correctly.

I can also see valid uses for both scenarios. Maybe there should be a third mode for compressHTML? Or an option similar to html-minifier's "Collapse whitespace" and "Collapse inline tag whitespace"

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-zwd8ub?file=src%2Fpages%2Findex.astro

Participation

  • I am willing to submit a pull request for this issue.
@Princesseuh
Copy link
Member

As far as I know, this is intended, compressHTML is never supposed to change the visual output of the website

@mayank99
Copy link
Contributor Author

mayank99 commented Mar 20, 2024

Isn't it already changing the visual output by removing indentation?

In any case, the current documentation is misleading.

And it would be useful to still have at least an opt-in mode where all whitespace is collapsed (similar to JSX). Currently this does not seem achievable even with a Vite plugin (transformIndexHtml does not get called on Astro-generated HTML).

@Princesseuh
Copy link
Member

No (well it's not supposed to at least?), because spaces in this context always get collapsed to one. That's why Prettier formats

<span>
  
  
	Something
  
  
  
</span>

into

<span> Something </span>

because they end up with the same visual result, since the number of whitespace doesn't matter.

And it would be useful to still have at least an opt-in mode where all whitespace is collapsed (similar to JSX). Currently this does not seem achievable even with a Vite plugin (transformIndexHtml does not get called on Astro-generated HTML).

I agree, that'd be cool! Essentially turning the HTML into JSX, ha.

@Princesseuh
Copy link
Member

Moving this issue to docs, since it's a docs issue in the end. It should be documented that compressHTML only compresses HTML in lossless manner and that in order to preserve the visual of the authored HTML, it might keep some whitespaces.

@Princesseuh Princesseuh transferred this issue from withastro/astro Mar 20, 2024
@Princesseuh
Copy link
Member

@mayank99 Feel free to create a roadmap discussion https://github.com/withastro/roadmap/discussions with the compressHTML idea!

@Princesseuh Princesseuh changed the title compressHTML doesn't remove all whitespace Document that compressHTML doesn't remove all whitespace Mar 20, 2024
@mayank99
Copy link
Contributor Author

mayank99 commented Mar 20, 2024

Yeah, that makes sense to me. Thanks for the quick response

Edit: created discussion withastro/roadmap#868

@TheOtterlord TheOtterlord added the improve documentation Enhance existing documentation (e.g. add an example, improve description) label Mar 20, 2024
@mayank99
Copy link
Contributor Author

mayank99 commented Apr 5, 2024

Worth also noting that the prettier plugin will sometimes forcibly insert meaningful whitespace, by breaking up long lines.

(If there was a way to trim whitespace, it wouldn't be an issue)

@sarah11918
Copy link
Member

Thanks for the discussion here!

Would be happy if someone wanted to make a PR to adjust the wording per Erika's note that

compressHTML only compresses HTML in lossless manner and that in order to preserve the visual of the authored HTML, it might keep some whitespaces.

The change must be made here (in the main astro repo):
https://github.com/withastro/astro/blob/main/packages/astro/src/%40types/astro.ts#L721

And maybe update the content to something like:

This is an option to minify your HTML output and reduce the size of your HTML files. Astro removes whitespace from your HTML, including line breaks, from .astro components in a lossless manner. This means that some whitespace may be kept to preserve the visual rendering of your HTML. This occurs both in development mode and in the final build. To disable HTML compression, set the compressHTML flag to false.

@sarah11918 sarah11918 added the help wanted Issues looking for someone to run with them! label May 4, 2024
@sarah11918 sarah11918 added the good first issue Good for newcomers label Jun 15, 2024
@sarah11918
Copy link
Member

Just pinging that we've got a good first issue here we'd still be happy if someone took on! Described what to do in the post above!

@Dillonpw
Copy link

@sarah11918 Id be willing to update this section of the docs later today if no one else has taken it on.

@sarah11918
Copy link
Member

@Dillonpw I don't believe anyone is on it! If you don't see a Docs PR for it, then please do go ahead. Thank you so much! 🚀

@sarah11918
Copy link
Member

This one was closed by the above PR!

@mayank99
Copy link
Contributor Author

I feel like the updated documentation is still insufficient and misleading. I mentioned earlier that the formatter can forcibly insert whitespace against the author's will. I've been able to work around it is using prettier-ignore comments, but it may not be obvious to other users.

@sarah11918
Copy link
Member

sarah11918 commented Jun 28, 2024

OK, so not just "fail to remove" some whitespace, but actually "add unexpected" whitespace? If the comment were updated to include that idea, would that be sufficient?

prettier plugin will sometimes forcibly insert meaningful whitespace

Just checking here which plugin: the Prettier plugin itself or Astro's plugin? (If Prettier, does it do this in non-Astro projects? Is this specifically a "how Prettier interacts with Astro" thing?)

@delucis
Copy link
Member

delucis commented Jun 28, 2024

If I understand @mayank99 correctly the issue is in two places:

  1. If you use a formatter like Prettier, it can add meaningful whitespace to your source files that changes rendering (seems like a bug in https://github.com/withastro/prettier-plugin-astro tbh)

  2. After that has happened, compressHTML won’t remove/fix that whitespace which the formatter added (which I guess is expected — Astro doesn’t know the extra whitespace was unintentional, so compresses it conservatively)

@mayank99
Copy link
Contributor Author

Yeah it's a prettier-plugin-astro issue, but I haven't even installed that directly. I believe it comes bundled with the Astro VSCode extension.

I'm not sure what the best way to document this would be, but yeah a brief mention would be nice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Issues looking for someone to run with them! improve documentation Enhance existing documentation (e.g. add an example, improve description)
Projects
None yet
Development

No branches or pull requests

6 participants