jaketrent / html-webpack-template

a better default template for html-webpack-plugin
MIT License
830 stars 139 forks source link

Reduce whitespace and output raw HTML where appropriate #68

Closed cameronhimself closed 6 years ago

cameronhimself commented 6 years ago

The initial goal of this PR was to reduce the whitespace in the generated HTML, however while plugging in values to test my changes, I noticed the following issues:

Both of these changes are contained in a separate commit, so I can make a new PR just for them, if you'd like.

Anyhow, onto the actual purpose of this PR: reducing whitespace. Here's the output before, with no options:

html-webpack-template-before

And here's after:

html-webpack-after

jaketrent commented 6 years ago

@cameronhimself thanks for this. I love the "after" results. So much nicer. Hopefully soon we'll all benefit from this change. Thanks for be patient while I got to this.

I checked out your code, went to examples/, npm install, npm start, which runs webpack-dev-server, and I get this error:

Html Webpack Plugin:

  Error: Child compilation failed:
  Module build failed: SyntaxError: Unexpected token -

  - Function

  - lodash.js:14856 
    [examples]/[lodash]/lodash.js:14856:16

  - lodash.js:465 apply
    [examples]/[lodash]/lodash.js:465:27

  - lodash.js:15240 
    [examples]/[lodash]/lodash.js:15240:16

  - lodash.js:467 apply
    [examples]/[lodash]/lodash.js:467:27

  - lodash.js:6575 
    [examples]/[lodash]/lodash.js:6575:16

  - lodash.js:14855 Function.template
    [examples]/[lodash]/lodash.js:14855:20

  - SyntaxError: Unexpected token -

  - compiler.js:76 
    [examples]/[html-webpack-plugin]/lib/compiler.js:76:16

  - Compiler.js:214 Compiler.
    [examples]/[webpack]/lib/Compiler.js:214:10

  - Compiler.js:403 
    [examples]/[webpack]/lib/Compiler.js:403:12

  - Tapable.js:67 Compiler.next
    [examples]/[tapable]/lib/Tapable.js:67:11

  - CachePlugin.js:40 Compiler.
    [examples]/[webpack]/lib/CachePlugin.js:40:4

  - Tapable.js:71 Compiler.applyPluginsAsync
    [examples]/[tapable]/lib/Tapable.js:71:13

  - Compiler.js:400 Compiler.
    [examples]/[webpack]/lib/Compiler.js:400:9

  - Compilation.js:577 Compilation.
    [examples]/[webpack]/lib/Compilation.js:577:13

  - Tapable.js:60 Compilation.applyPluginsAsync
    [examples]/[tapable]/lib/Tapable.js:60:69

  - Compilation.js:572 Compilation.
    [examples]/[webpack]/lib/Compilation.js:572:10

  - Tapable.js:60 Compilation.applyPluginsAsync
    [examples]/[tapable]/lib/Tapable.js:60:69

  - Compilation.js:567 Compilation.
    [examples]/[webpack]/lib/Compilation.js:567:9

  - Tapable.js:60 Compilation.applyPluginsAsync
    [examples]/[tapable]/lib/Tapable.js:60:69

  - Compilation.js:563 Compilation.
    [examples]/[webpack]/lib/Compilation.js:563:8

  - Tapable.js:60 Compilation.applyPluginsAsync
    [examples]/[tapable]/lib/Tapable.js:60:69

  - Compilation.js:525 Compilation.seal
    [examples]/[webpack]/lib/Compilation.js:525:7

Do you get this error? Any other? How'd you test this out?

I'm not sure what - is the offender in Unexpected token -. Looking at it, there are so many changes it's hard to tell. This error messages are the worst. :)

cameronhimself commented 6 years ago

Thanks for taking a peek at this, and sorry for not testing thoroughly. It looks like this is a classic case of putting the "ass" in "assumption". Since the file I was editing was an EJS file, I was using a command-line EJS compiler to test, which I assumed would be the functionally the same as the internal compiler used by html-webpack-plugin. It looks like that was not the case. This would also probably explain the issues I mentioned where inline HTML wasn't being escaped. I thought that was really weird, since surely someone else would have noticed that by now. I should have followed my instincts and kept digging.

Anyhow, I'll take another look and fix things up. My concern is that, without the whitespace-collapsing <%_ and -%> tags, the changes to the template to remove whitespace will make it very difficult to edit in the future, and it might not be a worthwhile trade-off. I'll try to make it as pretty as I can, but obviously it's your decision--no hard feelings either way.

cameronhimself commented 6 years ago

Done. I think I landed on a good strategy for maintaining readability and editability. The diff is kind of hard to read, but the basic idea was to change the structure from:

    <% if (foo) { %>
    <div>bar</div>
    <% } %>

    <% if (baz) { %>
    <div>bat</div>
    <% } %>

to:

    <% if (foo) { %>
    <div>bar</div><%
    } %><%

    if (baz) { %>
    <div>bat</div><%
    } %>

By putting the whitespace inside the EJS tags themselves, it allows for readability to be at least somewhat maintained without impacting the HTML output.


I tested properly this time, by going into examples and running npm install, npm start. I checked the output directly by running npx webpack. Here's what the example looks like out of the box:

webpack-html-default

And here it is after stripping down to the bare minimum config:

webpack-html-min

Anyhow, if you're good with this I'm happy to squash out the two false start commits for a cleaner history. Just let me know.

jaketrent commented 6 years ago

published in v6.2.0

pldg commented 6 years ago

I've tried to solve this whitespace problem with tags -%> (trims following newline) but doesn't work, anyone know how to make it works?

cameronhimself commented 6 years ago

@pldg The template file in html-webpack-template having the .ejs extension is a little misleading. The html-webpack-plugin does not use EJS by default--it uses a lodash template loader. See the docs. You can, however, configure webpack to use any loader you like.