inline-chunk-manifest-html-webpack-plugin icon indicating copy to clipboard operation
inline-chunk-manifest-html-webpack-plugin copied to clipboard

Add `inject` option

Open chaffeqa opened this issue 6 years ago • 1 comments

Supports:

  • false (as documented)
  • "body" (append to body of template)
  • "head" (append to head: default + original)

chaffeqa avatar Dec 12 '17 09:12 chaffeqa

Thank you for the PR 🥇

A couple of comments:

  1. The inject option you're referring to as documented is actually for html-webpack-plugin.
  • See this issue https://github.com/jouni-kantola/inline-chunk-manifest-html-webpack-plugin/issues/3
  • The following section in the readme: https://github.com/jouni-kantola/inline-chunk-manifest-html-webpack-plugin/blob/master/README.md#explicit-inject By using inject: false with html-webpack-plugin you can already inject the chunk manifest where you need it (i.e. body). With this in mind, do you still think this extra inject is a good configuration option? Let's discuss that for a bit.

After we decide if we should add this option or not, then I'd appreciate tests and docs for it.

  1. Tests fits around here: https://github.com/jouni-kantola/inline-chunk-manifest-html-webpack-plugin/blob/master/test/plugin-inject-manifest-test.js#L4

  2. The last bit that I think is missing is docs for the new configuration property. It probably needs extra clarity in this case, since there already is a similar configuration option for html-webpack-plugin.

jouni-kantola avatar Dec 12 '17 19:12 jouni-kantola