Making template compatible with html-webpack-plugin injection

[vladimyr]

🚧 This resolves #61 & #79

[jaketrent]

@vladimyr , thanks for giving this package some love and attention. I appreciate your cleanup and addressing some of the outstanding issues.

There is one breaking change that I've noticed that I'd like to revert on: that's the removal of arbitrary meta tags. I'd like to get that back. Are there other breaking changes that I'm not noticing? If so, I'd love to give those parts another review too.

The addition of the test suite is great! Is there a test that exercises the main premise of this PR, being able to inject via html-webpack-plugin?

If you're willing to make the meta change, I think we're close to a merge here. Thank you!

[jaketrent]

I would like to keep the ability to add meta tags as an array -- a user can add as many of the unanticipated kind of meta tags as they want. This package has drifted this direction as more and more people have wanted it to do more things: Just allow general setting of html via json.

[jaketrent]

Hey @vladimyr , how are you feeling about these changes that you've made (for the better of the project)? And about the requested adjustment for your meta implementation?

I'd love to get this merged in for all to enjoy.

[vladimyr]

Hi @jaketrent 👋

Sorry for being unresponsive. I had to shift my priorities but rest assured that I'm also eager to get it merged so community gets benefit from it but me too because I'm also using it inside my own projects.

There is one breaking change that I've noticed that I'd like to revert on: that's the removal of arbitrary meta tags. I'd like to get that back.

Um, I did that because html-webpack-plugin already supports meta injection (see meta option: https://github.com/jantimon/html-webpack-plugin#options) and my concern wasn't really duplicating functionality instead I'm not sure we can keep it around without conflicting with html-webpack-plugin's option. html-webpack-template's meta is an array while html-webpack-plugin expects dictionary object. I haven't tested it but my guess is that html-webpack-plugin won't pass meta down to the html-webpack-template. Instead it will fail due to wrong data format being provided? 🤔 Long story short, I need to do double check that but I think we are forced to make this change.

Are there other breaking changes that I'm not noticing? If so, I'd love to give those parts another review too.

I removed baseHref following the same logic - html-wepack-plugin already supports that through base option (see base option: https://github.com/jantimon/html-webpack-plugin#options). There aren't data format changes so it is merely matter of rename from baseHref to base.

Same applies to favicon that can be specified using html-webpack-plugin's favicon option (see favicon option: https://github.com/jantimon/html-webpack-plugin#options). It used to be a string, html-webpack-plugin expects a string, so no changes there.

I altered <link> tag generation to make it controllable by xhtml html-webpack-plugin's option (see xhtml option: https://github.com/jantimon/html-webpack-plugin#options). If xhtml compatibility mode is explicitly requested links will end up self closed, otherwise they'll be generated as usual. Previously links were always XHTML compatible.

I removed devServer option because injecting dev server client is webpack-dev-server's duty and is something that works out of box without requiring any template changes.

I removed inlineManifestWebpackName option because there is plugin listed inside html-webpack-plugin's docs that does exactly that so we don't need to keep that logic around.

I believe those are all breaking changes.

The addition of the test suite is great! Is there a test that exercises the main premise of this PR, being able to inject via html-webpack-plugin?

Well that is basically every test that has been added? Each test uses webpack programmatically and compiles spec/fixtures/index.js using dynamically generated webpack config that includes both html-webpack-plugin & html-webpack-template settings. Results (js bundle and generated html) are being written to in-memory filesystem and tested using regex patterns to verify that template settings are being correctly applied. They are basically e2e tests matching real life workflow and making sure html-webpack-template fits well there.

Hey @vladimyr , how are you feeling about these changes that you've made (for the better of the project)?

Great? 😁 Only reason I haven't turned this from draft to proper PR yet are missing documentation changes. Docs really need some love and having updating guide would be really nice. I have two ideas regarding docs:

1. Keep both old v6 and new v7? docs same way how webpack-chain does that: https://github.com/neutrinojs/webpack-chain#readme but I'm not sure how to do that as part of (single) PR. It requires creating new v6 branch (from current master) that readme could point to.
2. Use html-webpack-plugin's layout for describing options (https://github.com/jantimon/html-webpack-plugin#options) to reduce reader's cognitive load. I'm guessing that people will read those simultaneously so keeping it similar may help readers. What do you think?

Long story short I would personally like to see necessary documentation changes happen too. I'm willing to make those but can't guarantee on ETA. If you want you can merge it in current state so we make it 2-step upgrade or you can wait for me finishing it first which is totally up to you! 🎉

[jaketrent]

@vladimyr Thanks for the additional context on the changes you made. Please don't feel pressure on an ETA. This is all for the love of the project, right. Please take care of your other life priorities first.

Regarding the breaking changes, here's what I'm understanding: You removed meta, baseHref, and favicon because html-webpack-plugin already handles this directly. Great!

link change makes sense.

devServer and inlineManifestWebpackName removal because of handling elsewhere also makes sense.

With the additional info, I'm all for these changes.

I think what you point out is correct, we need a bit of documentation to point people in the right direction for these options if they were previously expecting to find them here.

And regarding your documentation points:

1. I'm not eager to get into the business of maintaining old and new docs. This might be nice, but I'd rather favor writing a one-time migration guide and encouraging upgrading.
2. Yes, I like the layout of the options format in html-webpack-plugin. I'm all for using that format.

Good luck as you get back around to this. If it sits for a long while, that's also just fine. Thanks again.