Webpack4 migrate #958

againksy · 2018-09-26T09:07:25Z

this is the migration to webpack 4 and react 16

change webpack config: 1- replace ExtractTextPlugin with MiniCssExtractPlugin 2- required packages updates 3- add production public path 4- everything is working as expected 5- will migrate to react 16

…ls, css-loader and webpack-dev-middleware

GGAlanSmithee · 2018-09-26T11:22:12Z

@againksy this is great, and well needed, thanks for taking the time to send this PR! I will have a look in the comming days! 🎉

GGAlanSmithee

Left a few comments, mostly cosmetic.

Seems like the application runs fine in prod, dev and test 👍

There are some warnings though:

Regarding styles

You are using @apply rule and custom property sets.
This feature won't be included in the next major release of postcss-cssnext.

Regarding code splitting / performance

WARNING in webpack performance recommendations:
You can limit the size of your bundles by using import() or require.ensure to lazy load some parts of your application.
For more info visit https://webpack.js.org/guides/code-splitting/

This might be an old warning though

Regarding lint

npm run lint

✖ 278 problems (278 errors, 0 warnings)
169 errors and 0 warnings potentially fixable with the --fix option.

GGAlanSmithee · 2018-09-28T05:52:06Z

app/tests/components/MainSection-test.js

 import MainSection from '../../components/MainSection';
 import TopicItem from '../../components/TopicItem';
+import Adapter from 'enzyme-adapter-react-16';


move above local imports

GGAlanSmithee · 2018-09-28T05:52:39Z

app/tests/setup.js

@@ -1,13 +1,16 @@
 require('babel-register') ({
  presets: ['es2015', 'react', 'stage-0']
 });
-var jsdom = require('jsdom').jsdom;
+var jsdom = require('jsdom');


var -> const

GGAlanSmithee · 2018-09-28T05:54:25Z

package.json

@@ -1,6 +1,6 @@
 {
  "name": "react-webpack-node",
-  "version": "3.4.1",
+  "version": "4.0.0",


not sure when it makes sense to bump a major version for this kind of a package, any other oppinions on this?

@GGAlanSmithee maybe 3.5.0 or some funny 3.4.4444 ?

@GGAlanSmithee what you think? I ll set it

GGAlanSmithee · 2018-09-28T05:55:58Z

webpack/paths.js

@@ -12,6 +12,7 @@ module.exports = {
  assets: path.resolve(CURRENT_WORKING_DIR, 'public', 'assets'),
  compiled: path.resolve(CURRENT_WORKING_DIR, 'compiled'),
  public: '/assets/', // use absolute path for css-loader?
+  prod_public: '/', // use absolute path for css-loader?


What does prod_public do? Cannot find any documentation regarding this.

@GGAlanSmithee in prod with public: '/assets/' the app tries to get the js and css from '/assets//assets/*' i did not searched why- just changed the prod publicPath for '/'.
I ll add comments about this.

GGAlanSmithee · 2018-09-28T05:56:19Z

webpack/plugins.js

@@ -1,5 +1,5 @@
 const webpack = require('webpack');
-const ExtractTextPlugin = require('extract-text-webpack-plugin');
+const MiniCssExtractPlugin = require("mini-css-extract-plugin");


GGAlanSmithee · 2018-09-28T05:56:49Z

webpack/plugins.js

      new ManifestPlugin({
        fileName: 'manifest.json'
      })
    ];
  }
  return [];
 };
+// migration to webpack 4


remove these comments before merge

GGAlanSmithee · 2018-09-28T05:57:47Z

webpack/rules/css.js

@@ -19,7 +19,7 @@ module.exports = ({ production = false, browser = false } = {}) => {
   *
   * Referenced from: https://github.com/webpack-contrib/css-loader#css-scope
   *
-   * For prerendering with extract-text-webpack-plugin you should use
+   * For prerendering with extract-text-webpack-plugin (replaced with mini-css-extract-plugin) you should use


normalize this comment, no need to mention old packages,

For prerendering with mini-css-extract-plugin ...

againksy · 2018-09-28T22:22:57Z

if we want to fix warning

You are using @apply rule and custom property sets.
css @apply is abandoned : https://www.xanthir.com/b4o00 , http://tabatkins.github.io/specs/css-apply-rule/
we should migrate from @apply- maybe you or someone know some alternative- how to replace this @apply with something?

GGAlanSmithee · 2018-10-01T06:13:38Z

server/db/mongo/constants.js

@@ -1,4 +1,5 @@
-export const db = process.env.MONGOHQ_URL || process.env.MONGODB_URI || 'mongodb://localhost/ReactWebpackNode';
+// export const db = process.env.MONGOHQ_URL || process.env.MONGODB_URI || 'mongodb://localhost/ReactWebpackNode';
+export const db = 'mongodb://reactgo_user_1:rgouser1pass@ds215093.mlab.com:15093/reactgo_sample';


revert this ;)

GGAlanSmithee · 2018-10-01T06:24:17Z

webpack/paths.js

@@ -12,7 +12,10 @@ module.exports = {
  assets: path.resolve(CURRENT_WORKING_DIR, 'public', 'assets'),
  compiled: path.resolve(CURRENT_WORKING_DIR, 'compiled'),
  public: '/assets/', // use absolute path for css-loader?
-  prod_public: '/', // use absolute path for css-loader?
+  prod_public: '/', // public path for production build should be like this


Thanks for adding the additional info. I do have a feeling that this is actually a problem with the rendering and/or webpack asset serving in dev mode (hmr etc9 and not the output parameter, I will take a look.

Also, If we keep these, let's try to inline the comments, something like:

public: '/assets/', prod_public: '/', // prod assets area already prefixed with '/assets/'

GGAlanSmithee · 2018-10-01T06:25:28Z

@againksy thanks for the update. Added a few more comments. Regarding @apply, I think we can leave this for now, and take it in a follow-up PR, since it's "only" a warning :)

GGAlanSmithee · 2018-10-12T06:29:16Z

@againksy this is great - thanks a million!

z and others added 5 commits September 26, 2018 11:12

migration to webpack 4

86d87e8

change webpack config: 1- replace ExtractTextPlugin with MiniCssExtractPlugin 2- required packages updates 3- add production public path 4- everything is working as expected 5- will migrate to react 16

react 16 update, udpate all packages, instead react-router, some babe…

6b1c10c

…ls, css-loader and webpack-dev-middleware

repo version 4

2909461

add package-lock

2d6acd4

set tested node engines v

c7aab7d

GGAlanSmithee requested changes Sep 28, 2018

View reviewed changes

newnewyear added 2 commits September 28, 2018 19:22

fx notes, 2 warnings left - post-css @apply and limit bundle sizes

a28c898

fx warning - only @apply css left. fx notes

4d3204f

GGAlanSmithee reviewed Oct 1, 2018

View reviewed changes

newnewyear added 2 commits October 1, 2018 21:20

fx notes

0545025

readme packages versions

03c2acf

GGAlanSmithee approved these changes Oct 12, 2018

View reviewed changes

GGAlanSmithee merged commit 1c97f65 into reactGo:master Oct 12, 2018

This was referenced Oct 12, 2018

New release / update changelog and publish to npm #959

Open

Consider bundle size limits for vendor packages #961

Open

Close / Update old / rotten PRs #962

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Webpack4 migrate #958

Webpack4 migrate #958

againksy commented Sep 26, 2018

GGAlanSmithee commented Sep 26, 2018

GGAlanSmithee left a comment

GGAlanSmithee Sep 28, 2018

GGAlanSmithee Sep 28, 2018

GGAlanSmithee Sep 28, 2018

againksy Sep 28, 2018

againksy Sep 28, 2018

GGAlanSmithee Sep 28, 2018

againksy Sep 28, 2018

GGAlanSmithee Sep 28, 2018

GGAlanSmithee Sep 28, 2018

GGAlanSmithee Sep 28, 2018

againksy commented Sep 28, 2018

GGAlanSmithee Oct 1, 2018

GGAlanSmithee Oct 1, 2018

GGAlanSmithee commented Oct 1, 2018

GGAlanSmithee commented Oct 12, 2018

Webpack4 migrate #958

Webpack4 migrate #958

Conversation

againksy commented Sep 26, 2018

GGAlanSmithee commented Sep 26, 2018

GGAlanSmithee left a comment

Choose a reason for hiding this comment

Regarding styles

Regarding code splitting / performance

Regarding lint

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

againksy commented Sep 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GGAlanSmithee commented Oct 1, 2018

GGAlanSmithee commented Oct 12, 2018