Improve webpack 4 support (afterEmit issue)

[andriijas]

No description provided.

[mastilver]

Thank you, so much ⭐

The failing test is there: https://github.com/danethurber/webpack-manifest-plugin/blob/master/spec/plugin.integration.spec.js#L74

[radum]

@andriijas You should remove Node 4 from Travis also. Node.js 4 is no longer supported. Source Code was upgraded to a higher ecmascript version.

[mastilver]

Yeah, good point! node@4 will be deprecated in April, so it's pretty safe to do it now

[codecov-io]

Codecov Report

Merging #128 into master will decrease coverage by 2.74%.
The diff coverage is 84.61%.

@@            Coverage Diff             @@
##           master     #128      +/-   ##
==========================================
- Coverage   94.89%   92.15%   -2.75%     
==========================================
  Files           2        2              
  Lines          98      102       +4     
==========================================
+ Hits           93       94       +1     
- Misses          5        8       +3

Impacted Files	Coverage Δ
lib/plugin.js	92.07% <84.61%> (-2.77%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 052970e...1935847. Read the comment docs.

[andriijas]

Call for help. 😱😱😱

I went through 💍 and 🔥 to get the tests working properly on all versions of webpack and extract text plugin and current status is that tests are running/failing randomly.

Check https://travis-ci.org/danethurber/webpack-manifest-plugin/builds/347232114

    TypeError: used is not a function

and sometimes

    SyntaxError: Unexpected end of JSON input

There is something Im not getting right with the async resolving but I cant figure it out. Please advise.

[mastilver]

Don't worry about SyntaxError: Unexpected end of JSON input it's just a racing condition issue (which we will be able to get rid once we stop supporting webpack < 3)

TypeError: used is not a function is a known issue mafintosh/mutexify#7, just leave the current version for mutexify. I'm getting rid of it anyway in #107 so you are good :)

[andriijas]

@mastilver okay cool. then the bigger question is whatever compiler.hooks.webpackManifestPluginAfterEmit should use a sync or async API

check 65f826d

I will squash the commits and force push when you have decided, dont want that ugly console.log in the history.

[mastilver]

I think we want it sync for the future (So we won't need the lock system) but I could be wrong

I'm sorry, I just merged something and there is some merge conflicts now... :/

[andriijas]

@mastilver rebased.

[aulisius]

Shouldn't this also be compiler.hooks.compilation.tap('ManifestPlugin', compilation) ?

[andriijas]

Nah thats the legacy for webpack <=3

[aulisius]

Ah, ok.

[andriijas]

@mastilver anything i need to adjust? would be so thankful for a new release!

[mastilver]

We need to figure out where the memory leak come from, I did some tweaking but I was unsuccessful... :/

[radum]

@mastilver They are all failing when extract text is in Beta, might be that is the issue.

[simonklee]

I tried this pull request with multiple instances of extract-text-webpack-plugin. It didn't work. Looks like it simply chose the last instance of extract-text-webpack-plugin and mapped that, while the first instance's output was discarded/overwritten.

[andriijas]

extract-text-webpack-plugin is now in maintainance mode for webpack 3.

Maybe we should add https://github.com/webpack-contrib/mini-css-extract-plugin for webpack 4 tests.

it will require some work though

[mastilver]

Oh, so extract-text-webpack-plugin won't ever be released for webpack 4?

I think we should just ignore that test for now... release v2 then release v3 with just support for webpack@4 (the failing test is something that is happening very rarely)

Maybe check if we are running webpack@4, then return from the test

What do you think?

[andriijas]

Sounds good👍

[rossta]

@mastilver Looks like extract-text-webpack-plugin support for webpack 4 support is being addressed on webpack-contrib/extract-text-webpack-plugin#685

[montogeek]

Please take a look at https://github.com/webpack-contrib/mini-css-extract-plugin, it was build for webpack 4 as a replacement for extract-text-webpack-plugin, please note that as its name says, it is intended for CSS extraction, no any text like extract-text-webpack-plugin.

[andriijas]

@rossta nope thats obsolete PR. check webpack-contrib/extract-text-webpack-plugin#749

@mastilver updated PR with some branching/if statements to detect webpack version in tests. Added test with MiniCSSExtractPlugin for webpack >=4

didnt have time for this but it had to be done 😂

[mastilver]

@andriijas Sorry, I'm opening a new PR for this, we can take care of the refactoring later on, I just want to get it working with webpack@4 ;)

Do you mind reviewing #134