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.

Sign up for GitHub

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

Jump to bottom

HTML Minification #7333

Closed
1 task
proyb6 opened this issue Jun 8, 2023 · 5 comments · Fixed by #7488
Closed
1 task

HTML Minification #7333

proyb6 opened this issue Jun 8, 2023 · 5 comments · Fixed by #7488
Assignees
@natemoo-re
Labels
- P2: nice to have Not breaking anything but nice to have (priority)

Comments

@proyb6
Copy link

proyb6 commented Jun 8, 2023 •
edited by Princesseuh
Loading

What version of astro are you using?

2.6.1

Are you using an SSR adapter? If so, which one?

Node

What package manager are you using?

npm

What operating system are you using?

macOS

What browser are you using?

Firefox

Describe the Bug

In 2.6.0 update which has been patched with
#7242

I found the <doctype> still contains newline while other part are minified, did the patch solved the issue?

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-wjgoff?file=astro.config.mjs

Participation

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

proyb6 commented Jun 8, 2023

<doctype> is minify when presender is set to true, export const prerender = true;

<doctype> is not minify when prerender is set to false.

@matthewp
Copy link
Contributor

matthewp commented Jun 8, 2023

Can you add a reproduction please?

@matthewp matthewp added the needs repro Issue needs a reproduction label Jun 8, 2023
@proyb6
Copy link
Author

proyb6 commented Jun 9, 2023 •
edited
Loading

@matthewp Code
https://stackblitz.com/edit/github-wjgoff?file=astro.config.mjs

HTML page will produce when the page is not set to prerender, doctype and HTML are still not minify:

<!DOCTYPE html>
<html lang="en"><head><meta charset="UTF-8"><meta name="description"...

I also notice the <script> tags are in new lines too.

<script>...
<script>...
<script>...

Prefer to test all the project in astro.new if you can.

@Princesseuh Princesseuh added - P2: nice to have Not breaking anything but nice to have (priority) and removed needs repro Issue needs a reproduction labels Jun 9, 2023
@proyb6
Copy link
Author

proyb6 commented Jun 13, 2023

Hopefully, a contributor can fix it.

@proyb6 proyb6 mentioned this issue Jun 18, 2023
Closed
1 task
@natemoo-re natemoo-re self-assigned this Jun 26, 2023
@natemoo-re
Copy link
Member

Looks like we aren't passing in the compressHTML setting when rendering a page in production. Figuring out how to make sure everything is wired up properly, but passing in isCompressHTML in these two places should fix it.

astro/packages/astro/src/core/app/index.ts

Line 255 in 0daaa9e

return renderPage({ mod, renderContext, env: this.#env, cookies: apiContext.cookies });

astro/packages/astro/src/core/app/index.ts

Line 263 in 0daaa9e

cookies: apiContext.cookies,

natemoo-re added a commit that referenced this issue Jun 26, 2023
@natemoo-re
fix(#7333): pass compressHTML to manifest
bfe5b80
@natemoo-re natemoo-re mentioned this issue Jun 26, 2023
Merged
natemoo-re added a commit that referenced this issue Jun 27, 2023
@natemoo-re
fix(#7333): pass compressHTML to manifest
6885e60
natemoo-re added a commit that referenced this issue Jun 28, 2023
@natemoo-re
Pass compressHTML settings to manifest (#7488)
d324785 
* fix(#7333): pass compressHTML to manifest

* chore: add compressHTML to astro:ssr-manifest test
matthewp pushed a commit that referenced this issue Jul 11, 2023
@natemoo-re @matthewp
Pass compressHTML settings to manifest (#7488)
d135b9c 
* fix(#7333): pass compressHTML to manifest

* chore: add compressHTML to astro:ssr-manifest test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@natemoo-re natemoo-re

Labels
- P2: nice to have Not breaking anything but nice to have (priority)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Pass compressHTML settings to manifest withastro/astro
4 participants
@matthewp @Princesseuh @natemoo-re @proyb6