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

HTML Minification #537

Closed
natemoo-re opened this issue Mar 23, 2023 · 12 comments
Closed

HTML Minification #537

natemoo-re opened this issue Mar 23, 2023 · 12 comments

Comments

@natemoo-re
Copy link
Member

Body

Summary

When building for production, we should safely collapse whitespace when generating the HTML

Background & Motivation

Astro users are often confused why we don't minify HTML by default.

The long answer is:

  • HTML minification can cause server/client hydration mismatches
  • Post-processing Astro's HTML output isn't possible in server mode
  • Streaming uses newlines to delineate when to flush buffered text in a response
  • Frameworks can use HTML comments as expression markers, which can cause issues if these are removed by a minifier

The shorter answer is:

  • It is possible, but we haven't made the time to do it properly yet

Goals

  • Safely (non-destructively) collapse whitespace during Astro compilation
  • Enable HTML minification in production by default
  • Allow users to enable/disable minification if they have specific needs

Non-Goals

  • Introduce a generic post-processing step for Astro markup
  • Future: option to control whether user comments should be present in the final markup

Example

Probably just a boolean config option. Name TBD.

@natemoo-re natemoo-re moved this to Stage 2: Accepted Proposals, No RFC in Public Roadmap Mar 23, 2023
@paperclover
Copy link

The compiler already has a flag that minifies markup from Astro components at compile time, which as far as I'm aware is unused in the framework.

I had a different discussion that briefly outlines how to get this method working, it's a couple of lines of code: #511. Not sure how much it applies given the "long answer."

@JerryWu1234
Copy link
Contributor

JerryWu1234 commented Mar 30, 2023

Do we need to support it as a new flag?
Such as pnpm run build --compact.
And by default is true?

@JerryWu1234
Copy link
Contributor

@matthewp cc

@JerryWu1234
Copy link
Contributor

JerryWu1234 commented Mar 31, 2023

@matthewp
Copy link
Contributor

I think we want a config option, yes. Probably build.compressHTML: true?

@JerryWu1234
Copy link
Contributor

I think we want a config option, yes. Probably build.compressHTML: true?

Ok, no problem.
I will continue when that bug of the compiler has been fixed.

@matthewp
Copy link
Contributor

@natemoo-re Where should the option for this go? I previously suggested build.compressHTML but how I realize this is both dev and build. Should this be top-level compressHTML?

@millette
Copy link

@matthewp Should the html actually be compressed when developing?

@JerryWu1234
Copy link
Contributor

JerryWu1234 commented Apr 13, 2023

@natemoo-re Where should the option for this go? I previously suggested build.compressHTML but how I realize this is both dev and build. Should this be top-level compressHTML?

I propose it only supports pro. so supporting build.compressHTML.
because the users don't need to see HTML that was compressed in most of the scenes in the dev.

@delucis
Copy link
Member

delucis commented Apr 13, 2023

In general, having it behave differently in dev vs production can trip developers up — if minification is the source of a bug, you don't see it until you get to a production or staging environment. So I'd suggest consistent dev & prod behaviour (which a user can then change by using a ternary in the config or passing a flag).

@JerryWu1234
Copy link
Contributor

In general, having it behave differently in dev vs production can trip developers up — if minification is the source of a bug, you don't see it until you get to a production or staging environment. So I'd suggest consistent dev & prod behaviour (which a user can then change by using a ternary in the config or passing a flag).

OK, I agree. I will follow what you said while I develop.

@matthewp matthewp moved this from Stage 2: Accepted Proposals, No RFC to Stage 3: Accepted Proposals, Has RFC in Public Roadmap May 8, 2023
@matthewp
Copy link
Contributor

matthewp commented May 19, 2023

This was released in 2.5.

@github-project-automation github-project-automation bot moved this from Stage 3: Accepted Proposals, Has RFC to Planned: Astro 3.0 in Public Roadmap May 19, 2023
@matthewp matthewp moved this from Planned: Astro 3.0 to Implemented in Public Roadmap May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Implemented
Development

No branches or pull requests

6 participants