-
-
Notifications
You must be signed in to change notification settings - Fork 184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add support for webpack4 #118
Conversation
Codecov Report
@@ Coverage Diff @@
## master #118 +/- ##
==========================================
- Coverage 100% 94.25% -5.75%
==========================================
Files 2 2
Lines 78 87 +9
==========================================
+ Hits 78 82 +4
- Misses 0 5 +5
Continue to review full report at Codecov.
|
Thank you very much for that! 👍 Can you edit the |
I updated it but I think we will have to wait until webpack-contrib/extract-text-webpack-plugin#707 to improve webpack 4 testing strategies. |
Cool tests are passing, I will merge that this week-end (remind me if I don't... ;) ) |
@mastilver Ping also on making a stable 2.0 release :) :) |
@mastilver When can we expect this to get merged? 😬 |
Also, webpack-contrib/extract-text-webpack-plugin#707 has been merged 🎉 |
For me its totally OK to upgade all dependencies, cut a 2.0 release of the plugin only working with webpack 4 and clean up old plugin API but I guess most people want to have a transition period. Just let me know if there is anything I can do to improve the PR. Thanks |
lib/plugin.js
Outdated
@@ -61,7 +59,7 @@ ManifestPlugin.prototype.apply = function(compiler) { | |||
path: path, | |||
chunk: chunk, | |||
name: name, | |||
isInitial: chunk.isInitial ? chunk.isInitial() : chunk.initial, | |||
isInitial: chunk.isInitial ? chunk.isInitial() : chunk.isOnlyInitial(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isInitial
is only in v3
.
So, I think l62
will throw error in v1/v2
. and it should be like this.
isInitial: chunk. isOnlyInitial ? chunk. isOnlyInitial() : (chunk.isInitial ? chunk.isInitial() : chunk.initial),
@andriijas Thank you for the great work, let me know if you want ot be more involved into this project! :) @ALL sorry for the delay, I didn't have much time. My boss just granted me permission to work on open source for the day, so I'm going to make the most out of it :) |
@mastilver Thank you for letting me, I honestly dont have any deep knowledge of webpack I tried my best using the migration guide: https://medium.com/webpack/webpack-4-migration-guide-for-plugins-loaders-20a79b927202 Im still not sure how callbacks changed with the new plugin API |
hum... weird, the build is now failing... maybe a dependency changed? |
Found where it's coming from, a test :) |
the test suite seems to run on webpack 3.x and maybe it should for a while. I read somewhere webpack 5 will be out in roughly 6 months, by then we can deprecate old plugin format. Maybe we should use tapAsync though if we want that callback support ref: facebook/create-react-app#4077 (comment) |
No, I'm talking about our unit tests and also I think we are missing: |
@mastilver check new PR |
Thank you @andriijas for your work, and all for your patience |
no worries, just been learning while helping. thanks finishing up! |
Please review carefully, Im not sure the PR is complete. I have no deep knowledge of webpack plugin internals.
I tested it in a small reference app and got a manifest generated.
Feedback appreciated.
Fixes #111