Append links HTML if no </head> tag is found

[khalwat]

As per the discussion here:

#134

Signed-off-by: Andrew Welch andrew@nystudio107.com

[coveralls]

Pull Request Test Coverage Report for Build 574

• 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.4%) to 91.837%

---

Totals
Change from base Build 559:	-0.4%
Covered Lines:	87
Relevant Lines:	88

---

💛 - Coveralls

[brunocodutra]

Thanks! Would you mind adding a unit test?

[khalwat]

I don't know how I could reasonably do it for this? Be happy to try if you guide me tho.

[khalwat]

@brunocodutra happy to do whatever it takes, I just need some guidance on what you need / how to implement it

[brunocodutra]

@khalwat sorry for the sparce response from my side, I'm currently on a trip.

You mentioned you decided to target the branch that supports v4 of html-webpack-plugin, is there anything preventing this PR from targeting master? The reason I ask is that currently the support for v4 of html-webpack-plugin can't be merged, because it lacks unit tests for that version, which may make it harder for you to add unit tests for your PR.

In any case, you should be able to replicate any of the currently existing unit tests. You will likely have to create a new fixture for the expected output of your changes, because it is expected to generate a completely different HTML from the other ones.

Let me know if you need specific guidance.

[khalwat]

It looks like the code isn't exactly the same for the html-webpack-plugin-4 branch:

master: https://github.com/brunocodutra/webapp-webpack-plugin/blob/master/src/index.js

        .then(result => {
          if (this.options.inject) {
            // Hook into the html-webpack-plugin processing and add the html
            tap(compilation, 'html-webpack-plugin-before-html-processing', 'WebappWebpackPlugin', (htmlPluginData, callback) => {
              const htmlPluginDataInject  = htmlPluginData.plugin.options.inject && htmlPluginData.plugin.options.favicons !== false;
              if ( htmlPluginDataInject || this.options.inject === 'force') {
                htmlPluginData.html = htmlPluginData.html.replace(/(<\/head>)/i, result + '$&');
              }
              return callback(null, htmlPluginData);
            });
          }
          return callback();
        })
        .catch(callback)

html-webpack-plugin-4: https://github.com/brunocodutra/webapp-webpack-plugin/blob/html-webpack-plugin-4/src/index.js

        .then(tags => {
          if (this.options.inject) {
            // Hook into the html-webpack-plugin processing and add the html
            tapHtml(compilation, 'WebappWebpackPlugin', (htmlPluginData, callback) => {
              const htmlPluginDataInject = htmlPluginData.plugin.options.inject && htmlPluginData.plugin.options.favicons !== false;
              if ( htmlPluginDataInject || this.options.inject === 'force') {
                htmlPluginData.html = htmlPluginData.html.replace(/(<\/head>)/i, tags.join('') + '$&');
              }
              return callback(null, htmlPluginData);
            });
          }
          return callback();
        })
        .catch(callback)

The primary difference is that the promise passes in the result (all of the tag HTML) for the master branch, and the tags (an array that contains each tag as separate items in the array). It's a relatively trivial difference.

[khalwat]

For my purposes, I'm only really interested in the html-webpack-plugin-4 since that's what I'm using... but I could make a PR vs. the master branch too if you like.

I've never created a fixture for unit tests for a JavaScript project before, though, so I'm probably not the best person to do it.

[brunocodutra]

These fixtures are really nothing fancy - simply generate the output as you would for any project and copy into that folder, that's it. Unit tests are supposed to generate the files in some temporary folder and compare (bitwise) them against the fixtures.

Don't worry about targeting master then, just bear in mind when generating the fixtures that unit tests still use html-webpack-plugin v3.

[khalwat]

Alrighty, I'm going to give it a go. Question: is html-webpack-plugin v3 compatible with webpack 4? I was going on the assumption that html-webpack-plugin v4 was needed for webpack 4 (thus the reason for me using it at all), but I think that assumption may be faulty?

If that's the case... then I could easily make the PR against master and table the PR for the html-webpack-plugin-4 branch until it has proper unit testing set up.

As I've never set up a JavaScript unit test in this fashion, I assume I'd just need to:

• Generate a fixture for the case where an empty template (or a template with no <head></head> is passed in
• Modify test/html.test.js to add a test for that case

Then my remaining question is... how to run the tests? I'm guessing it's as simple as just executing it with node?

[brunocodutra]

Alrighty, I'm going to give it a go

Awesome!

is html-webpack-plugin v3 compatible with webpack 4

Yes.

Generate a fixture for the case where an empty template (or a template with no <head></head> is passed in

Correct.

Modify test/html.test.js to add a test for that case

Either that or just create a new file just for it.

how to run the tests?

Tests need to be run through ava. The easiest is to run npm test and optionally temporarily change the test script in package.json to pass the single file in which you are interested so as to avoid wasting time testing everything over and over again.

[khalwat]

Looking good!

vagrant@homestead ~/webdev/node/webapp-webpack-plugin (master) $ npm test

> webapp-webpack-plugin@2.3.1 test /home/vagrant/webdev/node/webapp-webpack-plugin
> ava test/html.test.js



  3 tests passed

I'll close this PR and make a new one for the current master branch