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.

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

Bake-in support for webpack DLLs #1201

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Allow usage of webpack DLLs during development
One can now benefit from more build-time speed-ups by pre-compiling
vendor libraries into a DLL. To compile the DLL:

    npm run build-dlls

To use the DLL:

    export WEBPACK_DLLS=1 npm run dev

What isn't done:

- cache busting
- webpack isomorphic tools integration (if it's needed anyway, I don't
  use it/know it)
  • Loading branch information
amireh committed Jun 18, 2016
commit d5f6f12986aac10a87e90d787b20fabcdd5acf15
1 change: 1 addition & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"__SERVER__": true,
"__DISABLE_SSR__": true,
"__DEVTOOLS__": true,
"__DLLS__": true,
"socket": true,
"webpackIsomorphicTools": true
}
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ webpack-assets.json
webpack-stats.json
npm-debug.log
/.happypack
/webpack/dlls/manifests/*.json
1 change: 1 addition & 0 deletions bin/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ global.__CLIENT__ = false;
global.__SERVER__ = true;
global.__DISABLE_SSR__ = false; // <----- DISABLES SERVER SIDE RENDERING FOR ERROR DEBUGGING
global.__DEVELOPMENT__ = process.env.NODE_ENV !== 'production';
global.__DLLS__ = process.env.WEBPACK_DLLS === '1';

if (__DEVELOPMENT__) {
if (!require('piping')({
Expand Down
6 changes: 4 additions & 2 deletions karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ module.exports = function (config) {

files: [
'./node_modules/phantomjs-polyfill/bind-polyfill.js',
process.env.WEBPACK_DLLS === '1' && './static/dist/dlls/dll__vendor.js',
'tests.webpack.js'
],
].filter(function(x) { return !!x; }),

preprocessors: {
'tests.webpack.js': [ 'webpack', 'sourcemap' ]
Expand Down Expand Up @@ -53,7 +54,8 @@ module.exports = function (config) {
__CLIENT__: true,
__SERVER__: false,
__DEVELOPMENT__: true,
__DEVTOOLS__: false // <-------- DISABLE redux-devtools HERE
__DEVTOOLS__: false, // <-------- DISABLE redux-devtools HERE
__DLLS__: process.env.WEBPACK_DLLS === '1'
})
]
},
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"start-prod": "better-npm-run start-prod",
"start-prod-api": "better-npm-run start-prod-api",
"build": "better-npm-run build",
"build-dlls": "./node_modules/.bin/webpack --config webpack/dlls/vendor/webpack.config.js",
"postinstall": "npm run build",
"lint": "eslint -c .eslintrc src api",
"start-dev": "better-npm-run start-dev",
Expand Down
7 changes: 7 additions & 0 deletions src/helpers/Html.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@ export default class Html extends Component {
<body>
<div id="content" dangerouslySetInnerHTML={{__html: content}}/>
<script dangerouslySetInnerHTML={{__html: `window.__data=${serialize(store.getState())};`}} charSet="UTF-8"/>
{__DLLS__ && [
<script
key="dlls__vendor"
src="/dist/dlls/dll__vendor.js"
charSet="UTF-8"
/>
]}
<script src={assets.javascript.main} charSet="UTF-8"/>
</body>
</html>
Expand Down
10 changes: 8 additions & 2 deletions webpack/dev.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ var host = (process.env.HOST || 'localhost');
var port = (+process.env.PORT + 1) || 3001;
var HappyPack = require('happypack');
var happyThreadPool = HappyPack.ThreadPool({ size: 5 });
var WebpackHelpers = require('./helpers');

// https://github.com/halt-hammerzeit/webpack-isomorphic-tools
var WebpackIsomorphicToolsPlugin = require('webpack-isomorphic-tools/plugin');
Expand Down Expand Up @@ -63,7 +64,7 @@ reactTransform[1].transforms.push({
locals: ['module']
});

module.exports = {
var webpackConfig = module.exports = {
devtool: 'inline-source-map',
context: path.resolve(__dirname, '..'),
entry: {
Expand Down Expand Up @@ -129,7 +130,8 @@ module.exports = {
__CLIENT__: true,
__SERVER__: false,
__DEVELOPMENT__: true,
__DEVTOOLS__: true // <-------- DISABLE redux-devtools HERE
__DEVTOOLS__: true, // <-------- DISABLE redux-devtools HERE
__DLLS__: process.env.WEBPACK_DLLS === '1'
}),
webpackIsomorphicToolsPlugin.development(),

Expand All @@ -140,6 +142,10 @@ module.exports = {
]
};

if (process.env.WEBPACK_DLLS === '1') {
WebpackHelpers.installVendorDLL(webpackConfig, 'vendor');
}

// restrict loader to files under /src
function createSourceLoader(spec) {
return Object.keys(spec).reduce(function(x, key) {
Expand Down
31 changes: 31 additions & 0 deletions webpack/dlls/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# DLLs

Webpack "DLLs" can help reduce the build times a bit which is rather useful
during development.

You may opt-in to use the Vendor DLL by doing the following:

- set `WEBPACK_DLLS=1` in your shell before running webpack
- compile the DLL using `npm run build-dlls`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a little ambiguous, am I supposed to run WEBPACK_DLLS=1 npm run dev ? Is it only for development? Should there be specific npm tasks for running webpack with this env set?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need the flag when you're building using npm run build-dlls but you do need it set to use the DLLs, which is npm run dev and friends.

I personally think DLLs should only be used during development and had outlined my reasons in the main thread but it's not necessarily a must. I just don't see a gain in using it in a production environment as the prod build is already expected to be slow and doesn't run nearly as often as the dev one so.


You need to be careful to re-compile the DLL anytime a vendor module changes
(which is not often.)

## Defining a new DLL

See the `vendor` DLL under `/webpack/dlls/vendor/webpack.config.js` for
pointers on how to create one. Here are the guidelines:

- the DLL definition goes under `/webpack/dlls/[name]/webpack.config.js`
- the JS bundle goes under `/static/dist/dlls` with the name `dll__[name].js`
- the compiled manifest (auto-generated) goes under `/webpack/dlls/manifests`
with the name `[name].json`
- a reference to the DLL should be registered in `/webpack/dev.config.js`
- the build script `build-dlls` defined in `package.json` must be adjusted to compile that DLL
- the component `src/helpers/Html.js` should include the JS bundle
- `karma.conf.js` should preload the JS bundle under `files`

## Adding modules to the Vendor DLL

If you add a new dependency that you want to freeze within the DLL, add it
to `/webpack/dlls/vendor/webpack.config.js` under `entry.vendor`.
1 change: 1 addition & 0 deletions webpack/dlls/manifests/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Files in this directory are auto-generated. DO NOT EDIT!
83 changes: 83 additions & 0 deletions webpack/dlls/vendor/webpack.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
var path = require('path');
var webpack = require('webpack');
var root = path.resolve(__dirname, '..', '..', '..');

module.exports = {
devtool: process.env.NODE_ENV === 'production' ? null : 'inline-source-map',

output: {
path: path.join(root, 'static/dist/dlls'),
filename: 'dll__[name].js',
library: 'DLL_[name]_[hash]'
},

entry: {
vendor: [
'babel-polyfill',

// <babel-runtime>
//
// Generate this list using the following command against the stdout of
// webpack running against the source bundle config (dev/prod.js):
//
// egrep -o 'babel-runtime/\S+' | sed 's/\.js$//' | sort | uniq | wc -l
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be amended to include the full command to avoid the confusion (see main thread for context):

webpack \
  --config ./webpack/dev.config.js \
  --display-modules | egrep -o 'babel-runtime/\S+' | sed 's/\.js$//' | sort | uniq

'babel-runtime/core-js/array/from',
'babel-runtime/core-js/get-iterator',
'babel-runtime/core-js/is-iterable',
'babel-runtime/core-js/json/stringify',
'babel-runtime/core-js/number/is-integer',
'babel-runtime/core-js/number/is-safe-integer',
'babel-runtime/core-js/object/define-property',
'babel-runtime/core-js/object/get-own-property-descriptor',
'babel-runtime/core-js/object/get-own-property-names',
'babel-runtime/core-js/object/get-prototype-of',
'babel-runtime/core-js/promise',
'babel-runtime/helpers/create-class',
'babel-runtime/helpers/createClass',
'babel-runtime/helpers/defineProperty',
'babel-runtime/helpers/get',
'babel-runtime/helpers/possibleConstructorReturn',
'babel-runtime/helpers/slicedToArray',
'babel-runtime/helpers/to-consumable-array',
'babel-runtime/helpers/toConsumableArray',
// </babel-runtime>

'invariant',
'multireducer',
'react',
'react-bootstrap',
'react-dom',
'react-helmet',
'react-inline-css',
'react-redux',
'react-router',
'react-router-bootstrap',
'react-router-redux',
'redux',
'redux-async-connect',
'redux-form',
'scroll-behavior',
'serialize-javascript',
'socket.io-client',
'superagent',
'warning',
]
},

resolve: {
root: path.resolve(root, 'node_modules'),
extensions: [ '', '.js' ],
postfixes: [],
},

plugins: [
new webpack.DefinePlugin({
'process.env.NODE_ENV': JSON.stringify(process.env.NODE_ENV),
}),

new webpack.DllPlugin({
path: path.join(root, 'webpack/dlls/manifests/[name].json'),
name: 'DLL_[name]_[hash]'
})
]
};
47 changes: 47 additions & 0 deletions webpack/helpers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
var path = require('path');
var root = path.resolve(__dirname, '..');
var webpack = require('webpack');

exports.installVendorDLL = function(config, dllName) {
// DLL shizzle. Read more about this in /webpack/dlls/README.md
if (process.env.WEBPACK_DLLS === '1') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if statement is redundant at least in this specific use case because the calling code does the same check

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is, missed that one.

var manifest = loadDLLManifest(path.join(root, 'webpack/dlls/manifests/' + dllName + '.json'));

if (manifest) {
console.warn('Webpack: will be using the "%s" DLL.', dllName);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason this is a warning instead of a log?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a good reason at least!


config.plugins.push(new webpack.DllReferencePlugin({
context: root,
manifest: manifest
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little confused about this - can't manifest just be a string path to the file? Do you do this just to ensure the DLL exists before hand?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's what webpack expects; an object (the manifest) and not a path to where a manifest could be found. I could be wrong though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, my bad, you're right, I misread my own code

}));
}
}
};

function loadDLLManifest(filePath) {
try {
return require(filePath);
}
catch(e) {
process.env.WEBPACK_DLLS = '0';

console.error(
function() {/*
========================================================================
Environment Error
------------------------------------------------------------------------
You have requested to use webpack DLLs (env var WEBPACK_DLLS=1) but a
manifest could not be found. This likely means you have forgotten to
build the DLLs.

You can do that by running:

npm run build-dlls

The request to use DLLs for this build will be ignored.
*/}.toString().slice(15,-4)
);
}

return undefined;
}
3 changes: 2 additions & 1 deletion webpack/prod.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ module.exports = {
__CLIENT__: true,
__SERVER__: false,
__DEVELOPMENT__: false,
__DEVTOOLS__: false
__DEVTOOLS__: false,
__DLLS__: false,
}),

// ignore dev config
Expand Down
6 changes: 3 additions & 3 deletions webpack/webpack-isomorphic-tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ var WebpackIsomorphicToolsPlugin = require('webpack-isomorphic-tools/plugin');
// https://github.com/halt-hammerzeit/webpack-isomorphic-tools
module.exports = {

// when adding "js" extension to asset types
// when adding "js" extension to asset types
// and then enabling debug mode, it may cause a weird error:
//
// [0] npm run start-prod exited with code 1
// Sending SIGTERM to other processes..
//
// debug: true,
// debug: true,

assets: {
images: {
Expand Down Expand Up @@ -39,7 +39,7 @@ module.exports = {
// the only place it's used is the Html.js file
// where a <style/> tag is created with the contents of the
// './src/theme/bootstrap.config.js' file.
// (the aforementioned <style/> tag can reduce the white flash
// (the aforementioned <style/> tag can reduce the white flash
// when refreshing page in development mode)
//
// hooking into 'js' extension require()s isn't the best solution
Expand Down