Fix scope hoisting #152
Fix scope hoisting #152
Conversation
Codecov Report
@@ Coverage Diff @@
## master #152 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 3 3
Lines 110 111 +1
Branches 21 22 +1
=====================================
+ Hits 110 111 +1
Continue to review full report at Codecov.
|
spec/plugin.integration.spec.js
Outdated
| const plugins = [ | ||
| new ManifestPlugin(), | ||
| ]; | ||
| if (webpack.optimize.ModuleConcatenationPlugin) { |
There was a problem hiding this comment.
Can you use isWebpack4() so it's easy to refactor out once we drop webpack < 4
spec/plugin.integration.spec.js
Outdated
|
|
||
| describe('scoped hoisting', function() { | ||
| var compiler; | ||
| var hashes; |
There was a problem hiding this comment.
I don't think that's needed
spec/plugin.integration.spec.js
Outdated
| }); | ||
|
|
||
| afterAll(() => { | ||
| compiler.close() |
There was a problem hiding this comment.
I expect you only need that only on watch mode, no?
| ], | ||
| }, | ||
| output: { | ||
| filename: '[name].[hash].js', |
There was a problem hiding this comment.
Does it add value to add the hash?
| @@ -22,8 +22,10 @@ | |||
| "file-loader": "^1.1.11", | |||
| "jest": "^22.4.3", | |||
| "memory-fs": "^0.2.0", | |||
| "react": "^16.3.2", | |||
There was a problem hiding this comment.
I don't see it being used anywhere, is it needed?
There was a problem hiding this comment.
It's needed because svgr converts the svg file to a react component.
|
Thanks for the super fast review! If you get a chance to do a release after you merge this PR that would be doubly awesome! gaearon is asking for this as a follow up for CRA 2.0's webpack 4 support :) |
| it('outputs a manifest', function(done) { | ||
| let plugins; | ||
| if (webpack.optimize.ModuleConcatenationPlugin) { | ||
| // ModuleConcatenationPlugin works with webpack 3, 4. |
|
v2.0.3 released :) |
Issues: #151 facebook/create-react-app#4492
Same as
#117
with a test case.
fix #151
closes #117