[Feature] Webpack 4 upgrade #1562

rodeyseijkens · 2018-03-06T23:17:23Z

Updated for webpack 4
Upgraded general packages

Home chunk is now handled by webpack and it generates the best chunks together.
Each route has it own chunk now instead of the webpackMode: 'eager'.
Webpack for example creates a "home~about" and a "privacy~login" chunk and the server.js pushes the corresponding chunks to the SSR Html.js depending on the route it has.
This also eliminates the need for an array in the route.chunks, so this is renamed to route.chunk and a string now.

About the WIP part
There is still work that need needs to be done for the upcoming "DeprecationWarning: Tapable.plugin is deprecated. Use new API on .hooks instead". So in the start.js we use the compiler.plugin('compile') for example these still need to be changed to the new API.

If any of you have questions/enhancements please leave a comment
#1549

tim-soft · 2018-03-06T23:38:18Z

How about side-effects of static render -- anything need to change here?
https://github.com/kriasoft/react-starter-kit/blob/master/tools/render.js

rodeyseijkens · 2018-03-07T07:50:23Z

Shouldn’t be any different that before, so not affected.

rodeyseijkens · 2018-03-07T20:26:42Z

There is still a DeprecationWarning but that might be of one of the plugins, but not entirely sure.

"postcss-loader > schema-utils@0.4.3" has incorrect peer dependency "webpack@^2.0.0 || ^3.0.0"
postcss-loader loads for some reason schema-utils@0.4.3 instead of 0.4.5 which might be a fix but not 100% sure.

~~@tim-soft On the render question, it is broken, but I what I have seen is that it is even broken on the current master branch so this might need a new PR/Issue for a fix for that.~~

langpavel · 2018-03-07T20:29:07Z

I have seen is that it is even broken on the current master branch so this might need a new PR/Issue for a fix for that.

Can you describe really quickly what is broken?

rodeyseijkens · 2018-03-07T20:48:35Z

@langpavel sorry I thought the yarn render also created a build, but it isn't that was the confusion on my end. Forgot about the yarn build --release --static.

langpavel · 2018-03-07T20:51:45Z

@rodeyseijkens Yeah, this should be done some way better 😬 I was surprised by this too 🙂

rodeyseijkens · 2018-03-07T21:00:01Z

@langpavel new package.json script maybe?
"build-render": "yarn build --release --static"

rodeyseijkens · 2018-03-14T10:25:09Z

Latest commit removed NoEmitOnErrorsPlugin and NamedModulesPlugin cause its not needed or included in dev mode by default see: https://github.com/webpack/webpack/releases

I also pinned down the latest DeprecationWarning: Tapable.plugin, it is not in the RSK code. Its part of the AssetsPlugin. There is an issue open but I'm not entirely sure how fast they are going to fix it... cause it's been a year since an update on the package.

So what will be our status on this with this current PR?

tim-soft · 2018-03-15T15:43:34Z

Unfortunately this is the reality for a lot of webpack plugins right now, extract text plugin is keeping me from upgrading on a lot of projects.

Maybe there is an alternative that is v4 friendly such as https://github.com/webdeveric/webpack-assets-manifest

- Updated packages

rodeyseijkens · 2018-03-17T21:51:34Z

I implemented the webpack-assets-manifest as suggested by @tim-soft

So everything is now up to date for Webpack 4.0, so in my optinion it would be ready for merge.
@langpavel @koistya @frenzzy

langpavel

Great work. Thanks!

langpavel

route.chunks dropped?

langpavel · 2018-03-21T23:43:19Z

src/server.js

+        ...Object.keys(assets)
+          .filter(asset => asset.includes(route.chunk))
+          .map(chunk => assets[chunk].js),
+      );


@rodeyseijkens Why did you remove support for route.chunks?
May be wi can support both, chunk and chunks to keep this backward compatible?

rodeyseijkens · 2018-03-22T09:56:56Z

@langpavel the reason is that Webpack is now intelligent enough to split everything up in chunks that overlap thus you don't need to manually configure / figure out what chunks you want to combine. That is also why the 'eager' feature was dropped cause now it is exactly serving what is needed for that page and thus not any overloading of duplicate chunk data.

If we are going to manually put in more chunks it won't work because Webpack itself now figures out what chunks overlap and needs to be separate.

So for example you have a home, login, admin routes.
home.js will have its own chunk.
login.js will have its own chunk.
admin.js will have its own chunk.
home~login~admin.js is a shared chunk this will be generated by webpack itself cause it for example sees that a lot of componens or functions are shared (header component for example).
login~admin.js also ia a shared chunk and will be generated by webpack (input field component for example).
vendors~login~admin.js is a special vendor chunk specific for those pages (react-forms package that is only used on those pages) (possible when we have no vendor.js separation).

Hope this clears it up.

~~PS: There is still a bug I saw, it puts the .js.map files in the assets.json so they will be included in the SSR <script/> so need to fix that first.~~

tim-soft · 2018-03-22T10:59:22Z

Good catch! Is there a benefit to maintaining separate client and vendor bundles?

edit:
What I mean by separate is the browser will always need to grab these two bundles no matter what(?), so why split into two http requests

rodeyseijkens · 2018-03-22T14:27:52Z

@tim-soft Well kind of right I guess, maybe we don't need to have a special configuration then for Webpack splitChunks.

langpavel · 2018-03-22T16:35:59Z

@rodeyseijkens

If we are going to manually put in more chunks it won't work because Webpack itself now figures out what chunks overlap and needs to be separate.

I think you are wrong (you may be right but you should proof to me 🙂)
Loading multiple chunks can be implemented and should work IMHO

It is possible to have nested routes.
You can imagine that like page which have some tabs inside, like /admin/dashboard and /admin/articles.
Now you need admin chunk but you also need dashboard or articles chunk.
How you can do this with only one chunk? (Yes, I know, there can be potentially better way to do this, but not always I think)

langpavel · 2018-03-24T04:36:25Z

Merged 🎉

* Updated for webpack 4 * Updated start.js with webpack 4 hooks API * Removed NoEmitOnErrorsPlugin and NamedModulesPlugin * Updated Layout.test to have mocked pathname in context * Removed "assets-webpack-plugin" in favour of "webpack-assets-manifest" * Changed `vendor.js` → `vendors.js` — naming consistency with webpack * `assets.json` → `asset-manifest.json` * new file `chunk-manifest.json` for SSR support Closes kriasoft#1562, kriasoft#1549

rodeyseijkens added 2 commits March 6, 2018 23:41

Updated for webpack 4 & upgraded general packages

d308242

Comment cleanup

1f7e8d0

langpavel requested review from langpavel, koistya and frenzzy March 7, 2018 17:50

langpavel assigned langpavel, koistya, frenzzy and rodeyseijkens Mar 7, 2018

langpavel added enhancement for review in progress maintenance webpack labels Mar 7, 2018

Updated start.js with webpack 4 hooks API & updated some packages

c14162f

rodeyseijkens added 3 commits March 10, 2018 21:53

Updated packages

2a33673

Updated to latest LTS

5ef4909

Removed NoEmitOnErrorsPlugin and NamedModulesPlugin & Updated packages

4804cfc

pierreneter mentioned this pull request Mar 15, 2018

Update version of express-graphql #1567

Closed

rodeyseijkens added 2 commits March 15, 2018 12:55

Updated packages

2d85b4f

Updated Layout.test to have mocked pathname in context

51928ce

rodeyseijkens mentioned this pull request Mar 15, 2018

yarn test failed: src/components/Layout/Layout.test.js: Unexpected token (22:8) #1564

Closed

- Removed "assets-webpack-plugin" in favour of "webpack-assets-manifest"

d4505b4

- Updated packages

rodeyseijkens changed the title ~~[Feature][Working - WIP] Webpack 4 upgrade~~ [Feature] Webpack 4 upgrade Mar 17, 2018

langpavel approved these changes Mar 19, 2018

View reviewed changes

langpavel removed the in progress label Mar 19, 2018

langpavel mentioned this pull request Mar 20, 2018

Webpack 4 #1571

Merged

langpavel requested changes Mar 21, 2018

View reviewed changes

rodeyseijkens added 2 commits March 22, 2018 11:08

Changed vendor.js > vendors.js naming consistency with webpack

2ee1e0b

Filter .map files from assets.json

8459ded

Added .map file filter function

61c4f42

langpavel closed this in 84f9d5e Mar 24, 2018

rodeyseijkens deleted the feature/webpack-4-upgrade branch March 27, 2018 10:05

hajjiTarik mentioned this pull request Aug 1, 2022

[Snyk] Security upgrade node-fetch from 1.7.3 to 3.2.10 hajjiTarik/react-starter-kit#27

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] Webpack 4 upgrade #1562

[Feature] Webpack 4 upgrade #1562

rodeyseijkens commented Mar 6, 2018 •

edited

Loading

tim-soft commented Mar 6, 2018

rodeyseijkens commented Mar 7, 2018

rodeyseijkens commented Mar 7, 2018 •

edited

Loading

langpavel commented Mar 7, 2018

rodeyseijkens commented Mar 7, 2018

langpavel commented Mar 7, 2018 •

edited

Loading

rodeyseijkens commented Mar 7, 2018

rodeyseijkens commented Mar 14, 2018

tim-soft commented Mar 15, 2018

rodeyseijkens commented Mar 17, 2018

langpavel left a comment

langpavel left a comment

langpavel Mar 21, 2018

rodeyseijkens commented Mar 22, 2018 •

edited

Loading

tim-soft commented Mar 22, 2018 •

edited

Loading

rodeyseijkens commented Mar 22, 2018

langpavel commented Mar 22, 2018

langpavel commented Mar 24, 2018

[Feature] Webpack 4 upgrade #1562

[Feature] Webpack 4 upgrade #1562

Conversation

rodeyseijkens commented Mar 6, 2018 • edited Loading

tim-soft commented Mar 6, 2018

rodeyseijkens commented Mar 7, 2018

rodeyseijkens commented Mar 7, 2018 • edited Loading

langpavel commented Mar 7, 2018

rodeyseijkens commented Mar 7, 2018

langpavel commented Mar 7, 2018 • edited Loading

rodeyseijkens commented Mar 7, 2018

rodeyseijkens commented Mar 14, 2018

tim-soft commented Mar 15, 2018

rodeyseijkens commented Mar 17, 2018

langpavel left a comment

Choose a reason for hiding this comment

langpavel left a comment

Choose a reason for hiding this comment

langpavel Mar 21, 2018

Choose a reason for hiding this comment

rodeyseijkens commented Mar 22, 2018 • edited Loading

tim-soft commented Mar 22, 2018 • edited Loading

rodeyseijkens commented Mar 22, 2018

langpavel commented Mar 22, 2018

langpavel commented Mar 24, 2018

rodeyseijkens commented Mar 6, 2018 •

edited

Loading

rodeyseijkens commented Mar 7, 2018 •

edited

Loading

langpavel commented Mar 7, 2018 •

edited

Loading

rodeyseijkens commented Mar 22, 2018 •

edited

Loading

tim-soft commented Mar 22, 2018 •

edited

Loading