nuxt icon indicating copy to clipboard operation
nuxt copied to clipboard

perf(nuxt): ensure `renderHTMLDocument` return more compact result

Open Mini-ghost opened this issue 1 year ago โ€ข 5 comments

๐Ÿ”— Linked issue

Resolve #23085

โ“ Type of change

  • [ ] ๐Ÿ“– Documentation (updates to the documentation, readme or JSdoc annotations)
  • [ ] ๐Ÿž Bug fix (a non-breaking change that fixes an issue)
  • [x] ๐Ÿ‘Œ Enhancement (improving an existing functionality like performance)
  • [ ] โœจ New feature (a non-breaking change that adds functionality)
  • [ ] ๐Ÿงน Chore (updates to the build process or auxiliary tools and libraries)
  • [ ] โš ๏ธ Breaking change (fix or feature that would cause existing functionality to change)

๐Ÿ“š Description

I have attempted to minimize the size of the generated HTML by eliminating unnecessary line breaks where possible. Below is the HTML output from the current version after executing nuxi generate playground:

<!DOCTYPE html>
<html >
<head><meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<link rel="preload" as="fetch" crossorigin="anonymous" href="/_payload.json">
<link rel="modulepreload" as="script" crossorigin href="/_nuxt/entry.lirqHv3X.js">
<link rel="prefetch" as="script" crossorigin href="/_nuxt/error-404.tbGoJl1q.js">
<link rel="prefetch" as="script" crossorigin href="/_nuxt/vue.f36acd1f.eXCIzdvm.js">
<link rel="prefetch" as="script" crossorigin href="/_nuxt/error-500.9n_HQChl.js">
<script type="module" src="/_nuxt/entry.lirqHv3X.js" crossorigin></script></head>
<body ><div id="__nuxt"><div> Nuxt 3 Playground </div></div><script type="application/json" id="__NUXT_DATA__" data-ssr="true" data-src="/_payload.json">[{"state":1,"once":3,"_errors":5,"serverRendered":7,"path":8,"prerenderedAt":9},["Reactive",2],{},["Reactive",4],["Set"],["Reactive",6],{},true,"/",1703474852002]</script>
<script>window.__NUXT__={};window.__NUXT__.config={public:{},app:{baseURL:"/",buildAssetsDir:"/_nuxt/",cdnURL:""}}</script></body>
</html>

The adjusted versions are as follows:

<!DOCTYPE html><html><head><meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<link rel="preload" as="fetch" crossorigin="anonymous" href="/_payload.json">
<link rel="modulepreload" as="script" crossorigin href="/_nuxt/entry.HCIDZTxN.js">
<link rel="prefetch" as="script" crossorigin href="/_nuxt/error-404.fb9U4sbQ.js">
<link rel="prefetch" as="script" crossorigin href="/_nuxt/vue.f36acd1f.O0-pTQn_.js">
<link rel="prefetch" as="script" crossorigin href="/_nuxt/error-500.6TmM4WXY.js">
<script type="module" src="/_nuxt/entry.HCIDZTxN.js" crossorigin></script></head><body><div id="__nuxt"><div> Nuxt 3 Playground </div></div><script type="application/json" id="__NUXT_DATA__" data-ssr="true" data-src="/_payload.json">[{"state":1,"once":3,"_errors":5,"serverRendered":7,"path":8,"prerenderedAt":9},["Reactive",2],{},["Reactive",4],["Set"],["Reactive",6],{},true,"/",1703474792485]</script>
<script>window.__NUXT__={};window.__NUXT__.config={public:{},app:{baseURL:"/",buildAssetsDir:"/_nuxt/",cdnURL:""}}</script></body></html>

The extent of reduction is quite limited, primarily due to the fact that most of the <head> content is generated by @unhead/ssr, which might be beyond the scope of Nuxt.

I am open to feedback regarding whether this adjustment will yield significant benefits. Should the evaluation indicate that the drawbacks outweigh the benefits, please feel free to close this PR.

๐Ÿ“ Checklist

  • [x] I have linked an issue or discussion.
  • [ ] I have updated the documentation accordingly.

Mini-ghost avatar Dec 25 '23 03:12 Mini-ghost

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

CodSpeed Performance Report

Merging #24888 will degrade performances by 39.95%

:warning: No base runs were found

Falling back to comparing Mini-ghost:chore/reduce-document-size (6e879da) with main (1a9fb57)

Summary

โšก 1 improvements โŒ 3 regressions โœ… 4 untouched benchmarks

:warning: Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main Mini-ghost:chore/reduce-document-size Change
โŒ basic test fixture 451.9 ms 752.7 ms -39.95%
โŒ basic test fixture (types) 355 ms 533.5 ms -33.44%
โšก minimal test fixture (types) 136.3 ms 42.1 ms ร—3.2
โŒ minimal test fixture 39.2 ms 61.9 ms -36.67%

codspeed-hq[bot] avatar Dec 25 '23 04:12 codspeed-hq[bot]

re: extra spaces in <head > and <body >

Yes, please! Every time I see this I want to send a PR but have been avoiding the distraction

re: removing all line-breaks

In my experience, the saving % with compression isn't worth the loss of being able to easily debug page source. If you have benchmarks on the contrary then please share.

If you want smaller HTML then leaning into it and fully minifying the HTML will give you better results, which should already be easy to do via a nitro plugin.

Have no issues with this being an opt-in core feature though.

This is the same thinking I have with https://github.com/unjs/unhead/pull/297, which I'm happy to accept as an opt-in if we want to go in that direction.

harlan-zw avatar Dec 26 '23 06:12 harlan-zw

re: extra spaces in <head > and <body >

Yes, please! Every time I see this I want to send a PR but have been avoiding the distraction

re: removing all line-breaks

In my experience, the saving % with compression isn't worth the loss of being able to easily debug page source. If you have benchmarks on the contrary then please share.

If you want smaller HTML then leaning into it and fully minifying the HTML will give you better results, which should already be easy to do via a nitro plugin.

Have no issues with this being an opt-in core feature though.

This is the same thinking I have with unjs/unhead#297, which I'm happy to accept as an opt-in if we want to go in that direction.

Thank you for your valuable feedback.

However, I would like to seek some clarification: Are you suggesting the addition of an option in renderSSRHead or ssrRenderTags to remove newline characters?

Currently, I am integrating the Nitro plugin with html-minifier-terser to eliminate line-breaks and even to compact inline styles in my own projects. I believe it would be greatly beneficial if the core could support smaller HTML generation.

Nevertheless, I am curious about how the removal of newline characters might impact debugging, as I generally rely on Chrome DevTools for this purpose.

Mini-ghost avatar Dec 26 '23 09:12 Mini-ghost

I wonder if these changes will have a significant impact after text compression. GZIP/brotli should cover a lot of that already.

Would be interesting to see benchmarks on the standard application as well as "more real-life" project (e.g. something like https://github.com/nuxt/movies)

(As the refactoring here doesn't hurt readability significantly either I'm not opposed to merging btw)

Thank you for your feedback. I may need to explore how to test this change within the nuxt/movies context. I anticipate that the difference might not be very obvious. However, when combined with the enhancements in @unhead/ssr, the impact could potentially become more discernible.

Mini-ghost avatar Dec 26 '23 09:12 Mini-ghost

However, I would like to seek some clarification: Are you suggesting the addition of an option in renderSSRHead or ssrRenderTags to remove newline characters?

Yes, for both. Ideally, the head module would allow providing options to be used with the renderSSRHead function.

I think if you want to tackle this, you should do it in a separate PR and just keep this one simple, focusing on the extra unneeded spaces.

Nevertheless, I am curious about how the removal of newline characters might impact debugging, as I generally rely on Chrome DevTools for this purpose.

There are instances where you need to see the actual SSR DOM response instead of the client-hydrated DOM, especially when dealing with hydration issues.

harlan-zw avatar Dec 30 '23 01:12 harlan-zw

Yes, for both. Ideally, the head module would allow providing options to be used with the renderSSRHead function.

I think if you want to tackle this, you should do it in a separate PR and just keep this one simple, focusing on the extra unneeded spaces.

Sure! I will attempt to add an option to https://github.com/unjs/unhead/pull/297 that allows developers to decide whether to remove newline characters based on their needs.

Mini-ghost avatar Dec 30 '23 01:12 Mini-ghost

@manniL

I have tried to compare the differences before and after this PR using the Nuxt Movies project. From the Response Headers provided by Chrome DevTools, I found

Before Content-Length: 58148

After Content-Length: 58143

Perhaps this cannot be classified as an improvement. ๐Ÿ˜…

Mini-ghost avatar Jan 03 '24 18:01 Mini-ghost

@Mini-ghost Thanks for the effort! Yeah, not a huge thing but if it won't decrease perf or increase code complexity it is fine for me ๐Ÿ‘๐Ÿป

TheAlexLichter avatar Jan 03 '24 18:01 TheAlexLichter