numical / script-ext-html-webpack-plugin

Enhances html-webpack-plugin functionality with different deployment options for your scripts including 'async', 'preload', 'prefetch', 'defer', 'module', custom attributes, and inlining.
MIT License
588 stars 105 forks source link

1.7.2 release broke my config #18

Closed lukiffer closed 7 years ago

lukiffer commented 7 years ago

Confirmed that something between 1.7.0 and 1.7.2 broke. Nothing in the webpack config changed.

Our usage:

plugins: [new ScriptExtHtmlWebpackPlugin({
        defaultAttribute: 'defer'
      })]

Resulting error output:

ERROR in   TypeError: Cannot read property 'options' of undefined

  - plugin.js:35 Compilation.compilation.plugin
    [project-name/[script-ext-html-webpack-plugin]/lib/plugin.js:35:59

  - Tapable.js:206
    [project-name]/[tapable]/lib/Tapable.js:206:14

  - index.js:30 Compilation.<anonymous>
    [project-name]/[html-webpack-exclude-assets-plugin]/index.js:30:7

  - Tapable.js:208 Compilation.applyPluginsAsyncWaterfall
    [project-name]/[tapable]/lib/Tapable.js:208:13

  - util.js:16 Compilation.tryCatcher
    [project-name]/[bluebird]/js/release/util.js:16:23

  - index.js:150
    [project-name]/[html-webpack-plugin]/index.js:150:16

  - util.js:16 tryCatcher
    [project-name]/[bluebird]/js/release/util.js:16:23

  - promise.js:512 Promise._settlePromiseFromHandler
    [project-name]/[bluebird]/js/release/promise.js:512:31

  - promise.js:569 Promise._settlePromise
    [project-name]/[bluebird]/js/release/promise.js:569:18

  - promise.js:614 Promise._settlePromise0
    [project-name]/[bluebird]/js/release/promise.js:614:10

  - promise.js:693 Promise._settlePromises
    [project-name]/[bluebird]/js/release/promise.js:693:18

  - async.js:133 Async._drainQueue
    [project-name]/[bluebird]/js/release/async.js:133:16

  - async.js:143 Async._drainQueues
    [project-name]/[bluebird]/js/release/async.js:143:10

  - async.js:17 Immediate.Async.drainQueues
    [project-name]/[bluebird]/js/release/async.js:17:14
lukiffer commented 7 years ago

Were there breaking changes in the patch release? If so, are they documented somewhere? Cheers.

numical commented 7 years ago

Sorry! No. Can I see your webpack config - in particular the html webpack configuration. Also, what version of webpack?

lukiffer commented 7 years ago

Sure thing. https://gist.github.com/lukiffer/5b59c1c73b0002b49ead4cc78d0e604e

The configs get merged based on an environment argument. We're running webpack@2.2.0.

Thanks for investigating!

numical commented 7 years ago

Thans for that. I have a suspicion - can you remove HtmlWebpackExcludeAssetsPlugin from your config and check if the ScriptExt error gos away?

numical commented 7 years ago

To explain, both HtmlWebpackExcludeAssetsPlugin and ScriptExt use the same extension event from HtmlWebpackPlugin. If the correct args aren't passed along the chain, then my assumptions about the expected configuration could be wrong. If my suspicions are right simply removing HtmlWebpackExcludeAssetsPlugin from your config (as it is doing nothing) is a short term fix unitl I can release a more defensive version of ScriptExt...

lukiffer commented 7 years ago

So, yes that worked (no longer throwing the error), however we're doing an initial load of a stylesheet with core styles, and subsequently loading a theme stylesheet at runtime based on a localstorage preference or default. With HtmlWebpackExcludeAssetsPlugin commented out, all of the theme stylesheets are being <link>ed in index.html

I'm not sure if it was the exclusion of the other plugin or a change in ScriptExtHtmlWebpackPlugin thats causing that behavior.

numical commented 7 years ago

Excellent.
So first - it is a change in ScriptExt that has raised the issue. Though formally the issue is with HtmlWebpackExcludeAssetsPlugin not passing along HtmlWebpack data properly - I'll highlight this to them shortly. However this does not help you much in the short term so I will get a more defensivey coded version of ScriptExt out tonight - gimme until the morning please (it's dinnertime here!).

PS: I thought a no-arh instantiation of HtmlWebpackExcludeAssetsPlugin did nothing ...?

lukiffer commented 7 years ago

I believe the no-arg instantiation uses the HtmlWebpackPlugin data, in our case:

new HtmlWebpackPlugin({
        //...
        excludeAssets: [/theme-.*.css/]
})

It's no big rush - we've pinned 1.7.0 and it's still working well.

And btw, thank you for building and maintaining this package and being so responsive and easily accessible.

numical commented 7 years ago

Thankyou :-) v1.7.3 just published which should fix your issue. Not so easy to unit test (certainly not quickly) so do let me know if you still have difficulties.