From 3fc342e6e8695e492f01c2b4ad72bc77834c5368 Mon Sep 17 00:00:00 2001
From: Eric Fischer <efischer@edx.org>
Date: Wed, 25 Oct 2017 13:58:05 -0400
Subject: [PATCH] Webpack config prod/dev split

EDUCATOR-1448

Modifies paver webpack task to use production configuration in all
non-devstack environments, and a development config in devstack.
---
 cms/envs/common.py                            |  1 +
 cms/envs/devstack.py                          |  3 +
 common/static/common/js/karma.common.conf.js  |  2 +-
 lms/envs/common.py                            |  1 +
 lms/envs/devstack.py                          |  3 +
 package.json                                  |  7 ++-
 pavelib/assets.py                             | 20 ++++--
 pavelib/paver_tests/test_servers.py           | 12 ++--
 webpack.config.js => webpack.common.config.js | 58 +----------------
 webpack.dev.config.js                         | 53 ++++++++++++++++
 webpack.prod.config.js                        | 62 +++++++++++++++++++
 11 files changed, 153 insertions(+), 69 deletions(-)
 rename webpack.config.js => webpack.common.config.js (75%)
 create mode 100644 webpack.dev.config.js
 create mode 100644 webpack.prod.config.js

diff --git a/cms/envs/common.py b/cms/envs/common.py
index fd75914df19..4d06c8b30a4 100644
--- a/cms/envs/common.py
+++ b/cms/envs/common.py
@@ -814,6 +814,7 @@ WEBPACK_LOADER = {
         'STATS_FILE': os.path.join(STATIC_ROOT, 'webpack-stats.json')
     }
 }
+WEBPACK_CONFIG_PATH = 'webpack.prod.config.js'
 
 ################################# CELERY ######################################
 
diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py
index 56386e2ca69..b49f40631ca 100644
--- a/cms/envs/devstack.py
+++ b/cms/envs/devstack.py
@@ -48,6 +48,9 @@ STATICFILES_FINDERS = [
     'django.contrib.staticfiles.finders.AppDirectoriesFinder',
 ]
 
+# Load development webpack donfiguration
+WEBPACK_CONFIG_PATH = 'webpack.dev.config.js'
+
 ############################ PYFS XBLOCKS SERVICE #############################
 # Set configuration for Django pyfilesystem
 
diff --git a/common/static/common/js/karma.common.conf.js b/common/static/common/js/karma.common.conf.js
index 62e9dde3163..97e739e01f7 100644
--- a/common/static/common/js/karma.common.conf.js
+++ b/common/static/common/js/karma.common.conf.js
@@ -43,7 +43,7 @@ var _ = require('underscore');
 var appRoot = path.join(__dirname, '../../../../');
 var webdriver = require('selenium-webdriver');
 var firefox = require('selenium-webdriver/firefox');
-var webpackConfig = require(path.join(appRoot, 'webpack.config.js'));
+var webpackConfig = require(path.join(appRoot, 'webpack.dev.config.js'));
 
 delete webpackConfig.entry;
 
diff --git a/lms/envs/common.py b/lms/envs/common.py
index 2bbe2d05cee..541319c8b49 100644
--- a/lms/envs/common.py
+++ b/lms/envs/common.py
@@ -1801,6 +1801,7 @@ WEBPACK_LOADER = {
         'STATS_FILE': os.path.join(STATIC_ROOT, 'webpack-stats.json')
     }
 }
+WEBPACK_CONFIG_PATH = 'webpack.prod.config.js'
 
 ########################## DJANGO DEBUG TOOLBAR ###############################
 
diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py
index 9446c87e6cc..fb21fed0f14 100644
--- a/lms/envs/devstack.py
+++ b/lms/envs/devstack.py
@@ -112,6 +112,9 @@ REQUIRE_DEBUG = DEBUG
 
 PIPELINE_SASS_ARGUMENTS = '--debug-info'
 
+# Load development webpack donfiguration
+WEBPACK_CONFIG_PATH = 'webpack.dev.config.js'
+
 ########################### VERIFIED CERTIFICATES #################################
 
 FEATURES['AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING'] = True
diff --git a/package.json b/package.json
index 0c01aa58193..f3d36000489 100644
--- a/package.json
+++ b/package.json
@@ -13,9 +13,9 @@
     "coffee-loader": "^0.7.3",
     "coffee-script": "1.6.1",
     "css-loader": "^0.28.5",
-    "@edx/edx-bootstrap": "0.3.4",
-    "@edx/paragon": "0.1.0",
-    "@edx/studio-frontend": "0.2.0",
+    "@edx/edx-bootstrap": "^0.4.0",
+    "@edx/paragon": "^0.2.0",
+    "@edx/studio-frontend": "^0.3.0",
     "edx-pattern-library": "0.18.1",
     "edx-ui-toolkit": "1.5.2",
     "exports-loader": "^0.6.4",
@@ -45,6 +45,7 @@
     "underscore": "~1.8.3",
     "underscore.string": "~3.3.4",
     "webpack": "^2.2.1",
+    "webpack-merge": "^4.1.0",
     "webpack-bundle-tracker": "^0.2.0",
     "which-country": "1.0.0"
   },
diff --git a/pavelib/assets.py b/pavelib/assets.py
index 48eec72201a..3009844fbf5 100644
--- a/pavelib/assets.py
+++ b/pavelib/assets.py
@@ -765,12 +765,22 @@ def webpack(options):
     Run a Webpack build.
     """
     settings = getattr(options, 'settings', Env.DEVSTACK_SETTINGS)
+    static_root_lms = Env.get_django_setting("STATIC_ROOT", "lms", settings=settings)
+    static_root_cms = Env.get_django_setting("STATIC_ROOT", "cms", settings=settings)
+    config_path = Env.get_django_setting("WEBPACK_CONFIG_PATH", "lms", settings=settings)
     environment = 'NODE_ENV={node_env} STATIC_ROOT_LMS={static_root_lms} STATIC_ROOT_CMS={static_root_cms}'.format(
         node_env="production" if settings != Env.DEVSTACK_SETTINGS else "development",
-        static_root_lms=Env.get_django_setting("STATIC_ROOT", "lms", settings=settings),
-        static_root_cms=Env.get_django_setting("STATIC_ROOT", "cms", settings=settings),
+        static_root_lms=static_root_lms,
+        static_root_cms=static_root_cms
+    )
+    sh(
+        cmd(
+            '{environment} $(npm bin)/webpack --config={config_path}'.format(
+                environment=environment,
+                config_path=config_path
+            )
+        )
     )
-    sh(cmd('{environment} $(npm bin)/webpack'.format(environment=environment)))
 
 
 def execute_webpack_watch(settings=None):
@@ -782,7 +792,9 @@ def execute_webpack_watch(settings=None):
     # from Watchdog like the other watchers do.
     run_background_process(
         'STATIC_ROOT_LMS={static_root_lms} STATIC_ROOT_CMS={static_root_cms} $(npm bin)/webpack {options}'.format(
-            options='--watch --watch-poll=200',
+            options='--watch --watch-poll=200 --config={config_path}'.format(
+                config_path=Env.get_django_setting("WEBPACK_CONFIG_PATH", "lms", settings=settings)
+            ),
             static_root_lms=Env.get_django_setting("STATIC_ROOT", "lms", settings=settings),
             static_root_cms=Env.get_django_setting("STATIC_ROOT", "cms", settings=settings),
         )
diff --git a/pavelib/paver_tests/test_servers.py b/pavelib/paver_tests/test_servers.py
index 4048af4611a..026e4f71f5a 100644
--- a/pavelib/paver_tests/test_servers.py
+++ b/pavelib/paver_tests/test_servers.py
@@ -43,10 +43,12 @@ EXPECTED_INDEX_COURSE_COMMAND = (
 )
 EXPECTED_PRINT_SETTINGS_COMMAND = [
     u"python manage.py lms --settings={settings} print_settings STATIC_ROOT --format=value 2>/dev/null",
-    u"python manage.py cms --settings={settings} print_settings STATIC_ROOT --format=value 2>/dev/null"
+    u"python manage.py cms --settings={settings} print_settings STATIC_ROOT --format=value 2>/dev/null",
+    u"python manage.py lms --settings={settings} print_settings WEBPACK_CONFIG_PATH --format=value 2>/dev/null"
 ]
 EXPECTED_WEBPACK_COMMAND = (
-    u"NODE_ENV={node_env} STATIC_ROOT_LMS={static_root_lms} STATIC_ROOT_CMS={static_root_cms} $(npm bin)/webpack"
+    u"NODE_ENV={node_env} STATIC_ROOT_LMS={static_root_lms} STATIC_ROOT_CMS={static_root_cms} "
+    "$(npm bin)/webpack --config={webpack_config_path}"
 )
 
 
@@ -245,7 +247,8 @@ class TestPaverServerTasks(PaverTestCase):
             expected_messages.append(EXPECTED_WEBPACK_COMMAND.format(
                 node_env="production" if expected_asset_settings != Env.DEVSTACK_SETTINGS else "development",
                 static_root_lms=None,
-                static_root_cms=None
+                static_root_cms=None,
+                webpack_config_path=None
             ))
             expected_messages.extend(self.expected_sass_commands(system=system, asset_settings=expected_asset_settings))
         if expected_collect_static:
@@ -288,7 +291,8 @@ class TestPaverServerTasks(PaverTestCase):
             expected_messages.append(EXPECTED_WEBPACK_COMMAND.format(
                 node_env="production" if expected_asset_settings != Env.DEVSTACK_SETTINGS else "development",
                 static_root_lms=None,
-                static_root_cms=None
+                static_root_cms=None,
+                webpack_config_path=None
             ))
             expected_messages.extend(self.expected_sass_commands(asset_settings=expected_asset_settings))
         if expected_collect_static:
diff --git a/webpack.config.js b/webpack.common.config.js
similarity index 75%
rename from webpack.config.js
rename to webpack.common.config.js
index 35e89d66db6..4b1bebbb682 100644
--- a/webpack.config.js
+++ b/webpack.common.config.js
@@ -5,21 +5,14 @@
 var path = require('path');
 var webpack = require('webpack');
 var BundleTracker = require('webpack-bundle-tracker');
-var ExtractTextPlugin = require('extract-text-webpack-plugin');
 var StringReplace = require('string-replace-webpack-plugin');
 
-var isProd = process.env.NODE_ENV === 'production';
-var extractSass = new ExtractTextPlugin({
-    filename: 'css/[name].[contenthash].css',
-    disable: !isProd
-});
-
 var namespacedRequireFiles = [
     path.resolve(__dirname, 'common/static/common/js/components/views/feedback_notification.js'),
     path.resolve(__dirname, 'common/static/common/js/components/views/feedback.js')
 ];
 
-var wpconfig = {
+module.exports = {
     context: __dirname,
 
     entry: {
@@ -45,22 +38,12 @@ var wpconfig = {
 
     output: {
         path: path.resolve(__dirname, 'common/static/bundles'),
-        filename: isProd ? '[name].[chunkhash].js' : '[name].js',
         libraryTarget: 'window'
     },
 
-    devtool: isProd ? false : 'source-map',
-
     plugins: [
-        new ExtractTextPlugin('node_modules/@edx/studio-frontend/dist/studio-frontend.min.css'),
         new webpack.NoEmitOnErrorsPlugin(),
         new webpack.NamedModulesPlugin(),
-        new webpack.DefinePlugin({
-            'process.env.NODE_ENV': JSON.stringify(process.env.NODE_ENV || 'development')
-        }),
-        new webpack.LoaderOptionsPlugin({
-            debug: !isProd
-        }),
         new BundleTracker({
             path: process.env.STATIC_ROOT_CMS,
             filename: 'webpack-stats.json'
@@ -128,34 +111,6 @@ var wpconfig = {
                 ],
                 use: 'babel-loader'
             },
-            {
-                test: /(.scss|.css)$/,
-                include: [
-                    /studio-frontend/,
-                    /paragon/,
-                    /font-awesome/
-                ],
-                use: extractSass.extract({
-                    use: [{
-                        loader: 'css-loader',
-                        options: {
-                            modules: true,
-                            localIdentName: '[name]__[local]___[hash:base64:5]'
-                        }
-                    }, {
-                        loader: 'sass-loader',
-                        options: {
-                            data: '$base-rem-size: 0.625; @import "paragon-reset";',
-                            includePaths: [
-                                path.join(__dirname, './node_modules/@edx/paragon/src/utils'),
-                                path.join(__dirname, './node_modules/')
-                            ],
-                            sourceMap: true
-                        }
-                    }],
-                    fallback: 'style-loader'
-                })
-            },
             {
                 test: /\.coffee$/,
                 exclude: /node_modules/,
@@ -217,14 +172,3 @@ var wpconfig = {
         poll: true
     }
 };
-
-if (isProd) {
-    wpconfig.plugins = wpconfig.plugins.concat([
-        new webpack.LoaderOptionsPlugin({  // This may not be needed; legacy option for loaders written for webpack 1
-            minimize: true
-        }),
-        new webpack.optimize.UglifyJsPlugin()
-    ]);
-}
-
-module.exports = wpconfig;
diff --git a/webpack.dev.config.js b/webpack.dev.config.js
new file mode 100644
index 00000000000..1301fe782ad
--- /dev/null
+++ b/webpack.dev.config.js
@@ -0,0 +1,53 @@
+/* eslint-env node */
+
+'use strict';
+
+var Merge = require('webpack-merge');
+var path = require('path');
+var webpack = require('webpack');
+
+var commonConfig = require('./webpack.common.config.js');
+
+module.exports = Merge.smart(commonConfig, {
+    output: {
+        filename: '[name].js'
+    },
+    devtool: 'source-map',
+    plugins: [
+        new webpack.LoaderOptionsPlugin({
+            debug: true
+        }),
+        new webpack.DefinePlugin({
+            'process.env.NODE_ENV': JSON.stringify('development')
+        })
+    ],
+    module: {
+        rules: [
+            {
+                test: /(.scss|.css)$/,
+                include: [
+                    /studio-frontend/,
+                    /paragon/,
+                    /font-awesome/
+                ],
+                use: [{
+                    loader: 'css-loader',
+                    options: {
+                        modules: true,
+                        localIdentName: '[name]__[local]___[hash:base64:5]'
+                    }
+                }, {
+                    loader: 'sass-loader',
+                    options: {
+                        data: '$base-rem-size: 0.625; @import "paragon-reset";',
+                        includePaths: [
+                            path.join(__dirname, './node_modules/@edx/paragon/src/utils'),
+                            path.join(__dirname, './node_modules/')
+                        ],
+                        sourceMap: true
+                    }
+                }]
+            }
+        ]
+    }
+});
diff --git a/webpack.prod.config.js b/webpack.prod.config.js
new file mode 100644
index 00000000000..747d56c364e
--- /dev/null
+++ b/webpack.prod.config.js
@@ -0,0 +1,62 @@
+/* eslint-env node */
+
+'use strict';
+
+var Merge = require('webpack-merge');
+var path = require('path');
+var webpack = require('webpack');
+
+var ExtractTextPlugin = require('extract-text-webpack-plugin');
+var extractSass = new ExtractTextPlugin({
+    filename: 'css/[name].[contenthash].css'
+});
+
+var commonConfig = require('./webpack.common.config.js');
+
+module.exports = Merge.smart(commonConfig, {
+    output: {
+        filename: '[name].[chunkhash].js'
+    },
+    devtool: false,
+    plugins: [
+        new ExtractTextPlugin('node_modules/@edx/studio-frontend/dist/studio-frontend.min.css'),
+        new webpack.DefinePlugin({
+            'process.env.NODE_ENV': JSON.stringify('production')
+        }),
+        new webpack.LoaderOptionsPlugin({  // This may not be needed; legacy option for loaders written for webpack 1
+            minimize: true
+        }),
+        new webpack.optimize.UglifyJsPlugin()
+    ],
+    module: {
+        rules: [
+            {
+                test: /(.scss|.css)$/,
+                include: [
+                    /studio-frontend/,
+                    /paragon/,
+                    /font-awesome/
+                ],
+                use: extractSass.extract({
+                    use: [{
+                        loader: 'css-loader',
+                        options: {
+                            modules: true,
+                            localIdentName: '[name]__[local]___[hash:base64:5]'
+                        }
+                    }, {
+                        loader: 'sass-loader',
+                        options: {
+                            data: '$base-rem-size: 0.625; @import "paragon-reset";',
+                            includePaths: [
+                                path.join(__dirname, './node_modules/@edx/paragon/src/utils'),
+                                path.join(__dirname, './node_modules/')
+                            ]
+                        }
+                    }],
+                    fallback: 'style-loader'
+                })
+            }
+        ]
+    }
+});
-- 
GitLab