Switch to using 'inject: true' and not duplicating upstream functionality #61
Comments
|
@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. |
|
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? |
|
Hi! Sorry for the delayed reply. I'll try and take a look at a proof of concept PR this week :-) |
|
Hi @edmorley , did you manage to take a look on this? |
|
Without inject = true this plugin break other plugins. I played some hours with it and realize that I can use it. |
…1049) The HTML template that comes with `html-webpack-plugin` [1] is very minimal and so until now we've used the more fully-featured template from `html-webpack-template` [2] instead. However that template currently reimplements the core functionality of `html-webpack-plugin` in template markup and so has to be used with `inject` mode disabled [3]. This means: - many `html-webpack-plugin` plugins don't work (eg the one for SRI) - the template depends much more strongly on the internal data structures of `html-webpack-plugin`, and so can be broken by new major versions (as will be the case with `html-webpack-plugin` v4) - the template doesn't benefit from upstream improvements made to the injected content. In addition, `html-webpack-plugin` now supports several features that were previously only possible with a custom template (eg additional meta tags), and there are now many third party plugins to provide additional functionality (and are the preferred approach). As such, this switches `@neutrinojs/html-template` to using its own custom template, that is virtually the same as [1], other than adding `lang` and `appMountId` support. The `mobile` option has been replaced by adding the same viewport tag via the new `meta` option. If there are features that `html-webpack-template` supported, that our new simpler template does not, users can use their own template (`html-webpack-plugin` pushes this approach strongly) - Neutrino should cater for the 80% out of the box, not the 99%). Unrelated to the switch away from `html-webpack-template`, I've also stopped setting `xhtml` to `true`, since XHTML compliant output isn't something that most people need. [1] https://github.com/jantimon/html-webpack-plugin/blob/master/default_index.ejs [2] https://github.com/jaketrent/html-webpack-template/blob/master/index.ejs [3] jaketrent/html-webpack-template#61
|
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 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. |
|
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. |
I've filed #79 for this |
- removing `html-webpack-template` until jaketrent/html-webpack-template#61 gets resolved
The upstream
html-webpack-pluginplugin 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 jantimon/html-webpack-plugin@c8a6925).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:
html-webpack-plugin(as noted in FYI: Not compatible with HtmlWebpackInlineChunkPlugin #48), which will become increasingly important as more plugins become available.I would propose:
inject: true(this will require a major version bump)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.)
The text was updated successfully, but these errors were encountered: