Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Webpack 4 cont'd #4481

Closed
wants to merge 5 commits into from
Closed

Webpack 4 cont'd #4481

wants to merge 5 commits into from

Conversation

pelotom
Copy link
Contributor

@pelotom pelotom commented May 17, 2018

Work seems to have stalled on #4077 so I'm continuing it here. This applies andriijas#3.

andriijas and others added 4 commits May 15, 2018 15:10
@andriijas
- [x] Upgrade to webpack 4.8.X
e6fd0d1 
- [x] Utilize webpack 4 development and production modes
- [x] Upgrade webpack dev server
- [x] Webpack 4 compatible release of thread-loader
- [x] Webpack 4 compatible release of HtmlWebpackPlugin
- [x] Webpack 4 compatible release of SwPrecacheWebpackPlugin
- [x] Webpack 4 compatible release of WebpackManifestPlugin
- [x] Update README
- [x] Update WebpackDevServerUtils
- [x] Update InterpolateHtmlPlugin
- [x] Update ModuleScopePlugin
- [x] Update WatchMissingNodeModulesPlugin
- [x] Move UglifyJS options to webpack 4 optimize
- [x] Move InterpolateHtmlPlugin to make it tapable on HtmlWebpackPlugin
- [x] vendor splitting via splitChunks.splitChunks (https://twitter.com/wSokra/status/969633336732905474)
- [x] long term caching via splitChunks.runtimeChunk (https://twitter.com/wSokra/status/969679223278505985)
- [x] Make sure process.env.NODE_ENV is proxied correctly to `react-error-overlay`
- [x] Implicit webpack.NamedModulesPlugin in dev config as its default in webpack 4
- [x] Disable webpack performance hints as we have our own filesize reporter
- [x] Replace ExtractTextPlugin with MiniCssExtractPlugin
- [x] Switch to css whole file minification via OptimizeCSSAssetsPlugin rather than per module css minification to gain performance
Merge pull request facebook#1 from facebook/next
966ae83 
Map (css|sass|scss) modules to identity-obj-proxy in jest (facebook#4419)
disable module concatenation
543ae53
Merge branch 'webpack4' of https://github.com/bugzpodder/create-react…
2cbb719 
…-app into webpack4
@pelotom pelotom changed the title Webpack 4 Webpack 4 cont'd May 17, 2018
@bugzpodder
Copy link

I saw the timeout error from CI when running my test locally as well. but hey at least the yarn build succeeded

@gaearon
Copy link
Contributor

gaearon commented May 17, 2018

Looks like CSS is borked? Maybe it’s injected differently and the test relies on old implementation details?

f73cd777-1e5b-485e-a7ae-a94ca87273b5

@bugzpodder
Copy link

bugzpodder commented May 18, 2018 •
edited
Loading

So there are two issues here, as gaearon said the CSS is injected differently. MiniCssExtractPlugin writes css to a [name].chunk.css and uses link tag: <link href="http://www.example.org/spa/static/css/vendors.fd9f0736.chunk.css" rel="stylesheet">
This is a little bit hard to test to be honest. But at least its fixable as we know whats going on.

The second issue is very puzzling, when the CSS is not part of the vendors chunk (eg 27.e47f64ba.chunk.css) when running the mocha test the dynamic import (SassModuleInclusion.js) hangs causing a timeout.

@pelotom
Copy link
Contributor Author

pelotom commented May 18, 2018

@bugzpodder Do you have a fix for the first issue?

@bugzpodder
Copy link

I kinda of a fix for both. by kinda I mean not ideal.
For first issue we can technically read the actual file and check the contents.
For the second issue we can force webpack to bundle all the css together. this will make the test pass but I don't know whether it is actually a real issue. Not sure which underlying system broke (node, mocha, babel, or webpack). Probably have to run node_inspector to debug the actual test and see why the promise isn't resolving when importing chunk-ed CSS files...

@bugzpodder bugzpodder mentioned this pull request May 20, 2018
Merged
@gaearon
Copy link
Contributor

gaearon commented May 20, 2018

Thanks folks. I merged @bugzpodder's follow up PR to master.

@gaearon gaearon closed this May 20, 2018
@pelotom pelotom deleted the webpack4 branch May 21, 2018 17:20
@lock lock bot locked and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@pelotom @bugzpodder @gaearon @facebook-github-bot @andriijas