Skip to content
This repository has been archived by the owner on Nov 20, 2020. It is now read-only.

HTML Minification ? #12

Open
marceloboeira opened this issue Sep 19, 2014 · 18 comments
Open

HTML Minification ? #12

marceloboeira opened this issue Sep 19, 2014 · 18 comments
Labels

Comments

@marceloboeira
Copy link

I Guess it could be nice to minify html, with no cache of course.

@faeb187
Copy link

faeb187 commented Sep 20, 2014

Support for jade would be nice!! I think that's what you want ;)
For 'normal HTML' GZIP is supported in express-minify

BTW this is a feature request and not an issue.. please move

@marceloboeira
Copy link
Author

All feature requests are issues, not only problems, or bugs.
Just to be clear, html minification, with EJS or any other engine:

Code before:

<ul>
    <li>A</li>
    <li>B</li>
    <li>C</li>
    <li>D</li>
</ul>

Code after:

<ul><li>A</li><li>B</li><li>C</li><li>D</li></ul>

With any other render optimizations, of course.

@breezewish breezewish added the NFR label Sep 21, 2014
@faeb187
Copy link

faeb187 commented Sep 21, 2014

ah ok, thought issues are only bugs here, thx mate.
--> just did some tests.. you're right, the best practice seems to be minifying + gzip

Then it would be nice to have support for all of them:

  • Jade
  • EJS
  • plain HTML

@marceloboeira
Copy link
Author

@faeb187 weird. heh

@joeytwiddle
Copy link

I just want to warn, that occasionally HTML minification can change the meaning of a page! Specifically I experienced it when the browser was interpreting newlines as a gap, and the minifier removed those newlines (and indentation).

Given certain CSS settings (I forget which property now),

<span>
    Thing
</span>

is equivalent to:

<span> Thing </span>

But if you minify it to

<span>Thing</span>

then the resulting output is different! (You have lost the gap.)

For that reason, we stopped using Jade's built-in minifier for production, and stuck with un-minified HTML, like we had in development mode.

This is a rare issue, but it might trip people up (although if they use the minifier in both dev and prod mode, then they should be ok).

@joeytwiddle
Copy link

One other concern, if you are minifying but not caching, then you might be making the network transfer faster, but slowing down the actual response! This depends on the size of your HTML, and the speed of your minifier (related to CPU power of course). If you are caching semi-static pages, then this only applies to the first request, so no problem. And if it is optional, then the dev can make the decision for themself. Just wanted to warn about that too. ;)

@breezewish
Copy link
Owner

Minifying template is a good idea because it can be safely cached :-)

@marceloboeira
Copy link
Author

@joeytwiddle it could be several params, because there cases you may want to minify css inside <style> tags, or even javascript code on atributes...

cases like:

<a property="value"></a>

to

<a property=value></a>

or

<i class="icon-x"></i>

to

<i class="icon-x" />

....

@marceloboeira
Copy link
Author

I really want to minify, because I make a lot of requests with pjax, and all the HTML comes with spacing...

@breezewish
Copy link
Owner

@marceloboeira Have you enabled gzip? According to those real world test results, gzip is really an effective way to reduce traffic.

@marceloboeira
Copy link
Author

@breeswish yeap. but I guess html minification could help me even more.

@h2non
Copy link

h2non commented Mar 26, 2015

+1

@PeterDaveHello
Copy link
Contributor

+1 for html output minify

@iatek
Copy link

iatek commented Nov 18, 2015

  • 1 for html

@mcandre
Copy link

mcandre commented Jul 8, 2019

Bleugh, tired of waiting. Found https://www.npmjs.com/package/express-minify-html

It's not perfect, would love for regular express-minify to gain this ability...

@marceloboeira
Copy link
Author

this is oooooooooooooold as hell. I haven't done JS since 2015. hahah a

@mcandre
Copy link

mcandre commented Jul 18, 2019

express-minify-html's last commit was in 2017 and now features a high severity vulnerability.

Let's just add basic HTML minification to regular express-minify.

@Martii
Copy link

Martii commented Jul 18, 2019

Re: @mcandre

... and now features a high severity vulnerability.

Where would that be? e.g. no really.... citation is needed please.

Refs:

$ npm install
npm notice created a lockfile as package-lock.json. You should commit this file.
added 6 packages from 41 contributors and audited 7 packages in 0.524s
found 0 vulnerabilities

$ npm audit
                                                                                
                       === npm audit security report ===                        
                                                                                
found 0 vulnerabilities
 in 7 scanned packages

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

No branches or pull requests

9 participants