Switch to using 'inject: true' and not duplicating upstream functionality

[edmorley]

The upstream html-webpack-plugin plugin used to include most logic in the actual template file, however in v2.29.0 switched to injecting the relevant markup into the template using JS (see https://github.com/jantimon/html-webpack-plugin/commit/c8a69255b9fd2f66a41457340629b738f2e18936).

Currently html-webpack-template must be used with that JS injection mode turned off (inject: false), since it instead re-implements much of the upstream functionality in the custom index.ejs.

This results in a few issues:

• it breaks the plugin/event functionality of html-webpack-plugin (as noted in #48), which will become increasingly important as more plugins become available.
• it means the upstream functionality has to be re-implemented/duplicated here, and this package doesn't automatically benefit from any bug fixes/new features from upstream (which are likely more frequent given the popularity of the upstream package).
• it means there's a chance for slight behaviour differences for the features that exist both in upstream and this template, which could cause confusion when people switch between the two.

I would propose:

• updating the docs here to say to use inject: true (this will require a major version bump)
• removing the parts of index.ejs that are handled upstream, leaving just the custom funcationality

As an added bonus these changes will make it clearer what functionality this custom template adds, and then if in the future any of those features are upstreamed, this package can be simplified even further :-)

Thoughts? (If this sounds like something that would be accepted, I'm happy to open a PR.)

[jaketrent]

@edmorley thanks for outlining this. The situation and proposed direction are clear and helpful to me.

I love the idea of slimming this template and no duplicating upstream functionality. I know little about the fundamentals of what is broken and how changing inject true is going to fix it. Perhaps you could point me toward some basic education on the subject.

I'm in favor of moving in this direction and would welcome a PR.

[jaketrent]

Hey @edmorley I still like the idea of this -- not duplicating upstream functionality. I would welcome your expertise in adjusting it. Are you still interested? If not, no problem at all, and we can close this. What do you think?

[edmorley]

Hi! Sorry for the delayed reply. I'll try and take a look at a proof of concept PR this week :-)

[merlinnot]

Hi @edmorley , did you manage to take a look on this?

[alex-shamshurin]

Without inject = true this plugin break other plugins. I played some hours with it and realize that I can use it.

[edmorley]

Sorry for not having time to look at this. I believe it's something that's definitely worth doing, since in addition to the issues described in the OP, it caused breakage with this package when trying it with the new html-webpack-plugin v4 alpha. Those issues have since been fixed (html-webpack-plugin rolled back the breaking changes for now), however it proved how fragile inject: false is.

The Neutrino project has since switched away from using this package, so I probably won't have a chance to look at this - however happy to look over a PR if someone else does.

A first step to fixing this issue would be if someone could take a look at adding tests to this repository, that verified the output HTML for various combinations of options. Then when fixing this issue, it would be much easier to (a) review, (b) ensure no unexpected breakage.

[alex-shamshurin]

Right now I'm developing the same with handlebars engine and it will be possible to use it with "inject = true" and it much cleaner solution. All these "<% %>" impossible to read.

[edmorley]

A first step to fixing this issue would be if someone could take a look at adding tests to this repository,

I've filed #79 for this