-
-
Notifications
You must be signed in to change notification settings - Fork 178
Conversation
Codecov Report
@@ Coverage Diff @@
## master #369 +/- ##
=========================================
+ Coverage 97.69% 97.7% +0.01%
=========================================
Files 5 5
Lines 260 262 +2
Branches 102 103 +1
=========================================
+ Hits 254 256 +2
Misses 6 6
Continue to review full report at Codecov.
|
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.
Better name for option/function chunkFilter
, also it should be before .filter(ModuleFilenameHelpers.matchObject.bind(null, this.options))
, not first
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.
Better name for option/function chunkFilter
, also it should be before .filter(ModuleFilenameHelpers.matchObject.bind(null, this.options))
, not first
/cc @michael-ciniawsky what do you think? |
ef6edc4
to
2ddcec0
Compare
@evilebottnawi renamed to
Didn't understand this sorry. Can you provide a code snippet? I thought the chunk filter should go before the chunk file accumulator. |
@evilebottnawi @michael-ciniawsky any thoughts? |
@WearyMonkey Please read my review above and fix problem |
@evilebottnawi I'm only seeing:
I renamed
Didn't understand this sorry. Can you provide a code snippet? I thought the chunk filter should go before the chunk file accumulator. Am I missing something else? I think you commented during the github downtime so maybe something was lost. |
2ddcec0
to
5861728
Compare
We can't meaningfully disable optimization on the asmjs build until webpack-contrib/uglifyjs-webpack-plugin#369 is complete and it seems like optimization causes the browser to hang when it runs over the asmjs code
@evilebottnawi as far as I can tell I've responded to all your feedback. Am I missing something? |
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.
Looks good
Need fix test on windows |
/cc @WearyMonkey can you update snapshots, looks something wrong with cache on |
@evilebottnawi I've updated the snapshots on my mac, and tests are passing. Do I need to update them on Windows as well? |
@WearyMonkey will be great 👍 |
Adds a chunkFilter option to filter chunks. The existing test/include/exclude options are not useful when the filenames are generated at buildtime, e.g. `[hash].[ext]`.
5861728
to
c6e5348
Compare
@evilebottnawi I ran Not sure where to go from here? |
Update snapshots
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.
Need investigate, strange other PRs work good on CI
Close in favor #382, anyway thanks for the PR and helping to use do |
We can't meaningfully disable optimization on the asmjs build until webpack-contrib/uglifyjs-webpack-plugin#369 is complete and it seems like optimization causes the browser to hang when it runs over the asmjs code
We can't meaningfully disable optimization on the asmjs build until webpack-contrib/uglifyjs-webpack-plugin#369 is complete and it seems like optimization causes the browser to hang when it runs over the asmjs code
We can't meaningfully disable optimization on the asmjs build until webpack-contrib/uglifyjs-webpack-plugin#369 is complete and it seems like optimization causes the browser to hang when it runs over the asmjs code
Adds a chunkTest option to filter chunks.
This PR contains a:
Motivation / Use-Case
The existing test/include/exclude options are not useful when the filenames are generated at buildtime, e.g.
Breaking Changes
Additional Info
Related to: #21