Stencil 3250 - Webpack 2 w/ Code Splitting, Tree Shaking #964

mjschock · 2017-03-16T01:11:33Z

What?

Webpack 2 w/ Code Splitting, Tree Shaking, and other Optimizations

Tickets / Documentation

Depends On

STENCIL-286 - merged
STENCIL-3243 refactor stencil.conf.js to run as a worker #961 - merged

Notes

Stats

Before; size of bundle: 616K; size of zip: 10.3MB
After; total size of combined bundles: 612K; size of zip: 10.3MB
Note - if we get rid of code splitting (and async/await) the zip remains at 10.3MB but the bundle reduces to 560K (see the alternate diff here)

Code splitting example:
http://g.recordit.co/REeImIsk0o.gif

Appreciation

Thank you @dstaley for the incredibly useful writeup here.

@bigcommerce/stencil-team @bookernath

mjschock · 2017-03-16T01:12:09Z

assets/js/app.js

@@ -1,70 +1,50 @@
-import 'babel-polyfill';
+import 'babel-polyfill'; // TODO: can we remove this? see https://jira.bigcommerce.com/browse/MERC-684 and https://github.com/bigcommerce/cornerstone/commit/0648a3f900e093fb3ce8d30ddb087feb8e6b3c6d


want to see if we can actually remove this and just target es3/5 directly.

mjschock · 2017-03-16T01:13:54Z

webpack.conf.js

+    output: {
+        filename: '[chunkhash].theme-bundle.[name].js',
+        path: path.resolve(__dirname, "assets/dist"),
+        publicPath: 'assets/dist/', // TODO: cdnify - https://github.com/bigcommerce/paper/blob/master/index.js#L154


@mcampa - i'm wondering how we can make this work here. we want the publicPath to include the CDN for the code-split files. here's some more info: https://webpack.js.org/guides/public-path/

mjschock · 2017-03-16T01:14:14Z

webpack.conf.js

+                        cacheDirectory: true,
+                        presets: [['es2015', {loose: true, modules: false}]],
+                        plugins: [
+                            'syntax-dynamic-import', // do we need babel-plugin-dynamic-import-webpack?


didn't seem to need this but want to make sure...

mjschock · 2017-03-16T03:47:04Z

package.json

    "lazysizes": "3.0.0",
    "load-grunt-config": "0.17.1",
    "lodash": "^3.5.0",
    "nod-validate": "^2.0.12",
    "pace": "hubspot/pace#a03f1f1de62c9cea6c88b2267b8d7a83858b6cb6",
    "slick-carousel": "1.5.5",
    "time-grunt": "^1.2.2",
-    "webpack": "^1.14.0"
+    "webpack": "^2.2.1",
+    "webpack-visualizer-plugin": "^0.1.11"


TODO: remove this before merging

mjschock · 2017-03-16T03:47:53Z

stencil.conf.js

    webpackConfig.devtool = false;
-    webpackConfig.plugins.push(new webpack.optimize.DedupePlugin());


https://webpack.js.org/guides/migrating/#dedupeplugin-has-been-removed

mjschock · 2017-03-16T03:48:43Z

templates/layout/amp-iframe.ejs

+
+        {{#block "page"}} {{/block}}
+        <% for (var chunk in htmlWebpackPlugin.files.chunks) { %>
+        <script src="{{cdn '<%= htmlWebpackPlugin.files.chunks[chunk].entry %>'}}"></script>


now vendor and main are placed here on build, with hashes

mjschock · 2017-03-16T03:50:10Z

webpack.conf.js

+                    options: {
+                        compact: false,
+                        cacheDirectory: true,
+                        presets: [['es2015', {loose: true, modules: false}]],


modules: false needed for tree-shaking; not sure if we need loose, not sure what it was here for originally.

loose mode was for ie10: f6f2ec0

mjschock · 2017-03-20T18:17:00Z

assets/js/app.js

@@ -1,70 +1,51 @@
-import 'babel-polyfill';
+__webpack_public_path__ = window.__webpack_public_path__; // eslint-disable-line


this sets the public path for webpack to the base path for the cdn

mjschock · 2017-03-20T18:17:38Z

templates/layout/amp-iframe.html

-
-        <script src="{{cdn 'assets/dist/theme-bundle.js'}}"></script>
+
+        <script>window.__webpack_public_path__ = "{{cdn 'assets/dist/'}}";</script>


the cdn base path is needed for the dynamically loaded bundles

mjschock · 2017-03-20T18:17:51Z

webpack.conf.js

    bail: true,
    module: {
-        loaders: [
+        rules: [


https://webpack.js.org/guides/migrating/#module-loaders-is-now-module-rules

mjschock · 2017-03-20T18:18:11Z

webpack.conf.js

-                    cacheDirectory: true,
-                    presets: ['es2015-loose'],
+                use: {
+                    loader: 'babel-loader',


https://webpack.js.org/guides/migrating/#automatic-loader-module-name-extension-removed

mjschock · 2017-03-20T18:18:41Z

stencil.conf.js

+            warnings: true,
+        },
+        minimize: true,
+        sourceMap: true,


https://webpack.js.org/guides/migrating/#uglifyjsplugin-sourcemap

mjschock · 2017-03-20T18:18:55Z

stencil.conf.js

-        comments: false
+        comments: false,
+        compress: {
+            warnings: true,


https://webpack.js.org/guides/migrating/#uglifyjsplugin-warnings

mjschock · 2017-03-20T18:40:09Z

webpack.conf.js

+        }),
+        new webpack.optimize.CommonsChunkPlugin({
+            name: 'vendor',
+            minChunks: function (module) {


https://webpack.js.org/plugins/commons-chunk-plugin/#passing-the-minchunks-property-a-function

mjschock · 2017-03-20T19:16:32Z

stencil.conf.js

+        compress: {
+            warnings: true,
+        },
+        sourceMap: true,


https://webpack.js.org/guides/migrating/#uglifyjsplugin-sourcemap

If this plugin is only used to bundle for production we don't include the source maps

it's off now, with a comment

mjschock · 2017-03-20T19:17:47Z

webpack.conf.js

+            watch: false
+        }),
+        new webpack.LoaderOptionsPlugin({
+             minimize: true


https://webpack.js.org/guides/migrating/#uglifyjsplugin-minimize-loaders

mjschock · 2017-03-21T21:53:21Z

assets/js/app.js


+const getAccount = () => import('./theme/account');
+const getLogin = () => import('./theme/auth');


these two functions are shared

mjschock · 2017-03-21T21:53:51Z

browserlist

@@ -0,0 +1,8 @@
+# See https://support.bigcommerce.com/articles/Public/Themes-Supported-Browsers/


perhaps these should go back 3 versions to match the autoprefixer settings?

Yes, change it to last 3

mjschock · 2017-03-21T21:55:00Z

stencil.conf.js

-    webpackConfig.plugins.push(new webpack.optimize.DedupePlugin());
-    webpackConfig.plugins.push(new webpack.optimize.UglifyJsPlugin({ comments: false }));
+    webpackConfig.plugins.push(new webpack.LoaderOptionsPlugin({
+        minimize: true,


https://webpack.js.org/guides/migrating/#uglifyjsplugin-minimize-loaders

mjschock · 2017-03-21T21:55:44Z

stencil.conf.js

+    webpackConfig.plugins.push(new webpack.optimize.UglifyJsPlugin({
+        comments: false,
+        compress: {
+            warnings: true,


https://webpack.js.org/guides/migrating/#uglifyjsplugin-warnings

mjschock · 2017-03-21T21:56:38Z

templates/layout/amp-iframe.html

@@ -24,7 +24,10 @@

        {{#block "page"}} {{/block}}

-        <script src="{{cdn 'assets/dist/theme-bundle.js'}}"></script>
+        <script>window.__webpack_public_path__ = "{{cdn 'assets/dist/'}}";</script>


this is used (in apps.js) to prefix the dynamically loaded chunk requests

mjschock · 2017-03-21T21:58:32Z

webpack.conf.js

+                return module.context && module.context.indexOf('node_modules') !== -1;
+            },
+        }),
+        new webpack.optimize.CommonsChunkPlugin({


this one pulls duplicate child files out of the code split chunks into their parent

mjschock · 2017-03-21T21:59:01Z

stencil.conf.js

@@ -41,8 +41,16 @@ function development() {
 function production() {
    webpackConfig.watch = false;
    webpackConfig.devtool = false;
-    webpackConfig.plugins.push(new webpack.optimize.DedupePlugin());


https://webpack.js.org/guides/migrating/#dedupeplugin-has-been-removed

mjschock · 2017-03-21T22:00:15Z

webpack.conf.js

+    resolve: {
+        alias: {
+            jquery: path.resolve(__dirname, 'node_modules/jquery/dist/jquery.min.js'),
+            jstree: path.resolve(__dirname, 'node_modules/jstree/dist/jstree.min.js'),


both package.json's for jquery and jstree point to the their non-minified versions, this resolves to the minified versions instead

mjschock · 2017-03-21T22:01:20Z

webpack.conf.js

+            name: 'vendor',
+            minChunks: function (module) {
+                // this assumes your vendor imports exist in the node_modules directory
+                return module.context && module.context.indexOf('node_modules') !== -1;


pull out the node_modules into the vendor bundle (https://webpack.js.org/plugins/commons-chunk-plugin/#passing-the-minchunks-property-a-function)

mcampa · 2017-03-22T00:38:36Z

assets/js/app.js

                // Finds the appropriate class from the pageType.
-                const PageClass = pageClasses[templateFile];
+                const templatePath = pageClasses[templateFile];


Can you rename templatePath to something else? since is not a path anymore

sure, how about pageClassImporter?

mjschock · 2017-03-22T17:24:39Z

Cornerstone-1.6.2.zip

nickdengler · 2017-03-30T17:32:39Z

LGTM 💚

Ping me when it goes to integration so I can submit a PR for failing tests (https://github.com/bigcommerce/bigcommerce/pull/18253)

mjschock · 2017-03-30T18:30:30Z

Cornerstone-1.6.3.zip

mcampa

👍

mjschock commented Mar 16, 2017

View reviewed changes

mjschock mentioned this pull request Mar 17, 2017

WIP - Stencil 3245 #972

Closed

mjschock commented Mar 20, 2017

View reviewed changes

mjschock changed the title ~~WIP - Stencil 3250~~ Stencil 3250 - Webpack 2 w/ Code Splitting Mar 20, 2017

mjschock commented Mar 20, 2017

View reviewed changes

mjschock mentioned this pull request Mar 21, 2017

WIP - Stencil 3244 #973

Closed

mjschock changed the title ~~Stencil 3250 - Webpack 2 w/ Code Splitting~~ Stencil 3250 - Webpack 2 w/ Code Splitting, Tree Shaking, Optimization Mar 21, 2017

mjschock changed the title ~~Stencil 3250 - Webpack 2 w/ Code Splitting, Tree Shaking, Optimization~~ Stencil 3250 - Webpack 2 w/ Code Splitting, Tree Shaking Mar 21, 2017

mjschock commented Mar 21, 2017

View reviewed changes

mcampa reviewed Mar 22, 2017

View reviewed changes

junedkazi approved these changes Mar 30, 2017

View reviewed changes

mcampa approved these changes Apr 25, 2017

View reviewed changes

webpack 2

b2b9007

mjschock merged commit ff212a0 into bigcommerce:master Apr 25, 2017

mjschock deleted the STENCIL-3250 branch April 25, 2017 19:43

ghost mentioned this pull request Jan 9, 2018

Navigation Menu #962

Closed

		@@ -1,70 +1,50 @@
		import 'babel-polyfill';
		import 'babel-polyfill'; // TODO: can we remove this? see https://jira.bigcommerce.com/browse/MERC-684 and https://github.com/bigcommerce/cornerstone/commit/0648a3f900e093fb3ce8d30ddb087feb8e6b3c6d

		webpackConfig.devtool = false;
		webpackConfig.plugins.push(new webpack.optimize.DedupePlugin());

		@@ -1,70 +1,51 @@
		import 'babel-polyfill';
		__webpack_public_path__ = window.__webpack_public_path__; // eslint-disable-line


		<script src="{{cdn 'assets/dist/theme-bundle.js'}}"></script>

		<script>window.__webpack_public_path__ = "{{cdn 'assets/dist/'}}";</script>


		const getAccount = () => import('./theme/account');
		const getLogin = () => import('./theme/auth');

		@@ -0,0 +1,8 @@
		# See https://support.bigcommerce.com/articles/Public/Themes-Supported-Browsers/

Stencil 3250 - Webpack 2 w/ Code Splitting, Tree Shaking #964

Stencil 3250 - Webpack 2 w/ Code Splitting, Tree Shaking #964

Conversation

mjschock commented Mar 16, 2017 • edited Loading

What?

Tickets / Documentation

Depends On

Notes

Stats

Appreciation

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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjschock commented Mar 22, 2017

nickdengler commented Mar 30, 2017

mjschock commented Mar 30, 2017

mcampa left a comment

Choose a reason for hiding this comment

mjschock commented Mar 16, 2017 •

edited

Loading