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

Upgrade to webpack 2 #2380

Merged
merged 30 commits into from
May 17, 2017
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
2294658
first pass at webpack 2 upgrade
kumar303 May 10, 2017
631a9a2
upgrade extract-text-webpack-plugin
kumar303 May 10, 2017
1979568
removed unnecessary plugins, fixed defaults for the new uglify
kumar303 May 10, 2017
a688738
upgraded loader chains
kumar303 May 10, 2017
999cf94
fixed karma webpack config
kumar303 May 10, 2017
488aba2
converted loader query strings to option objects
kumar303 May 11, 2017
84ec3ec
fixed postcss options
kumar303 May 11, 2017
f323abe
resolve __dirname, why not?
kumar303 May 11, 2017
142d1b7
DRY up module.rules
kumar303 May 12, 2017
99be0a7
DRY up plugins
kumar303 May 12, 2017
c52d771
Throw a better error message here
kumar303 May 12, 2017
ed1308b
fix syntax error
kumar303 May 12, 2017
eff7d10
Switch to a fork of the sri-stats-webpack-plugin plugin
kumar303 May 12, 2017
0f096fb
building and running the app locally both require env vars
kumar303 May 12, 2017
d093fd9
Upgrade babel-loader
kumar303 May 12, 2017
79ea4ed
fix lint errors
kumar303 May 12, 2017
0becc82
better comments about replacement plugins
kumar303 May 15, 2017
732a201
better shared file loader config
kumar303 May 15, 2017
19d7e96
mime types are inferred so this can DRY up even more
kumar303 May 15, 2017
8ec66ce
move devServer configuration to dev config file
kumar303 May 15, 2017
3eb2ff6
update yarn.lock
kumar303 May 15, 2017
eb720de
Replaced sri-stats-webpack-plugin with webpack-subresource-integrity
kumar303 May 16, 2017
cec2440
Merge branch 'master' into webpack2-iss1761
kumar303 May 16, 2017
5ac680f
update yarn.lock
kumar303 May 16, 2017
e3f3895
Simplify SriDataPlugin
kumar303 May 16, 2017
ee7faab
removing comment, uplift looks unlikely
kumar303 May 17, 2017
12d0079
document progress so far on running production mode locally
kumar303 May 17, 2017
6291206
Merge branch 'master' into webpack2-iss1761
kumar303 May 17, 2017
0f3865e
Better docs for running a local production build
kumar303 May 17, 2017
69f3e4d
Allow local-production* config files
kumar303 May 17, 2017
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
7 changes: 4 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -238,9 +238,10 @@ The env vars are:

Example: Building and running a production instance of the admin app:

```
NODE_APP_INSTANCE=admin NODE_ENV=production npm run build && npm run start
```
````
NODE_APP_INSTANCE=amo NODE_ENV=production npm run build
NODE_APP_INSTANCE=amo NODE_ENV=production npm run start
````

## What version is deployed?

Expand Down
3 changes: 2 additions & 1 deletion config/default.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ const validAppNames = [

// Throw if the appName supplied is not valid.
if (appName && !validAppNames.includes(appName)) {
throw new Error(`App ${appName} is not enabled`);
throw new Error(
`App "${appName}" is not enabled; valid app names: ${validAppNames}`);
}


Expand Down
71 changes: 11 additions & 60 deletions karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,12 @@ require('babel-register');

const fs = require('fs');

const webpack = require('webpack');
const config = require('config');

const getClientConfig = require('./src/core/utils').getClientConfig;
const webpackCommon = require('./webpack-common');
const webpackConfigProd = require('./webpack.prod.config.babel').default;

const clientConfig = getClientConfig(config);
const getPlugins = webpackCommon.getPlugins;
const getRules = webpackCommon.getRules;

const babelrc = fs.readFileSync('./.babelrc');
const babelQuery = JSON.parse(babelrc);

Expand All @@ -24,16 +23,14 @@ const coverageReporters = [{
babelQuery.plugins.push(['istanbul', { include: 'src/**' }]);

const newWebpackConfig = Object.assign({}, webpackConfigProd, {
devtool: 'inline-source-map',
module: {
rules: getRules({ babelQuery, bundleStylesWithJs: true }),
},
output: undefined,
entry: undefined,
plugins: [
new webpack.DefinePlugin({
CLIENT_CONFIG: JSON.stringify(clientConfig),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the shared getPlugins() array, the process.env.NODE_ENV value is added to DefinePlugin unlike before. This doesn't seem to cause any problems though.

}),
// Replaces server config module with the subset clientConfig object.
new webpack.NormalModuleReplacementPlugin(/config$/, 'core/client/config.js'),
// Substitutes client only config.
new webpack.NormalModuleReplacementPlugin(/core\/logger$/, 'core/client/logger.js'),
// Use the browser's window for window.
new webpack.NormalModuleReplacementPlugin(/core\/window/, 'core/browserWindow.js'),
...getPlugins({ excludeOtherAppLocales: false }),
// Plugin to show any webpack warnings and prevent tests from running
// Based on: https://gist.github.com/Stuk/6b574049435df532e905
function WebpackWarningPlugin() {
Expand Down Expand Up @@ -69,52 +66,6 @@ const newWebpackConfig = Object.assign({}, webpackConfigProd, {
});
},
],
devtool: 'inline-source-map',
module: {
loaders: [
{
test: /\.jsx?$/,
exclude: /node_modules/,
loader: 'babel',
query: babelQuery,
}, {
test: /\.css$/,
loader: 'style!css?importLoaders=2!postcss?outputStyle=expanded',
}, {
test: /\.scss$/,
loader: 'style!css?importLoaders=2!postcss!sass?outputStyle=expanded',
}, {
test: /\.svg$/,
loader: 'url?limit=10000&mimetype=image/svg+xml',
}, {
test: /\.jpg$/,
loader: 'url?limit=10000&mimetype=image/jpeg',
}, {
test: /\.png$/,
loader: 'url?limit=10000&mimetype=image/png',
}, {
test: /\.gif/,
loader: 'url?limit=10000&mimetype=image/gif',
}, {
test: /\.webm$/,
loader: 'url?limit=10000&mimetype=video/webm',
}, {
test: /\.mp4$/,
loader: 'url?limit=10000&mimetype=video/mp4',
}, {
test: /\.otf$/,
loader: 'url?limit=10000&mimetype=application/font-sfnt',
}, {
test: /\.woff$/,
loader: 'url?limit=10000&mimetype=application/font-woff',
}, {
test: /\.woff2$/,
loader: 'url?limit=10000&mimetype=application/font-woff2',
},
],
},
output: undefined,
entry: undefined,
});

const reporters = [
Expand Down
11 changes: 6 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@
"dompurify": "0.8.9",
"es6-error": "4.0.2",
"express": "4.15.2",
"extract-text-webpack-plugin": "1.0.1",
"extract-text-webpack-plugin": "2.1.0",
"fastclick": "1.0.6",
"helmet": "3.6.0",
"humps": "2.0.0",
Expand Down Expand Up @@ -229,7 +229,7 @@
"babel-core": "^6.24.1",
"babel-eslint": "^7.2.3",
"babel-gettext-extractor": "git+https://github.com/muffinresearch/babel-gettext-extractor.git#0d39d3882bc846e7dcb6c9ff6463896c96920ce6",
"babel-loader": "^6.4.1",
"babel-loader": "^7.0.0",
"babel-plugin-istanbul": "^4.1.3",
"babel-plugin-react-transform": "^2.0.2",
"babel-plugin-transform-class-properties": "^6.24.1",
Expand Down Expand Up @@ -285,17 +285,18 @@
"semver": "^5.3.0",
"shelljs": "^0.7.7",
"sinon": "^2.2.0",
"sri-stats-webpack-plugin": "^0.7.3",
"style-loader": "^0.17.0",
"stylelint": "^7.10.1",
"stylelint-config-standard": "^16.0.0",
"supertest": "^3.0.0",
"supertest-as-promised": "^4.0.2",
"svg-url-loader": "^2.0.2",
"tmp": "^0.0.31",
"tosource": "^1.0.0",
"webpack": "^1.14.0",
"webpack": "^2.5.1",
"webpack-dev-middleware": "^1.10.2",
"webpack-dev-server": "^2.4.5",
"webpack-hot-middleware": "^2.18.0"
"webpack-hot-middleware": "^2.18.0",
"webpack-subresource-integrity": "^1.0.0-rc.1"
}
}
2 changes: 1 addition & 1 deletion src/core/client/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export default function makeClient(routes, createStore) {
try {
if (locale !== langToLocale(config.get('defaultLang'))) {
// eslint-disable-next-line max-len, global-require, import/no-dynamic-require
require(`bundle?name=[name]-i18n-[folder]!../../locale/${locale}/${appName}.js`)(renderApp);
require(`bundle-loader?name=[name]-i18n-[folder]!../../locale/${locale}/${appName}.js`)(renderApp);
} else {
renderApp({});
}
Expand Down
39 changes: 39 additions & 0 deletions src/core/server/sriDataPlugin.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// This is a webpack plugin to expose the data generated by SriPlugin.
// See webpack.prod.config.babel.js
import fs from 'fs';

import { oneLine } from 'common-tags';

// TODO: hopefully we can uplift this.
// See https://github.com/waysact/webpack-subresource-integrity/issues/45

export default class SriDataPlugin {
constructor({ saveAs } = {}) {
if (!saveAs) {
throw new Error('The saveAs parameter cannot be empty');
}
this.saveAs = saveAs;
}

apply(compiler) {
compiler.plugin('done', (stats) => {
const sriStats = {};
try {
Object.keys(stats.compilation.assets).forEach((baseName) => {
const asset = stats.compilation.assets[baseName];
if (!asset.integrity) {
throw new Error(
oneLine`The integrity property is falsey for
asset ${baseName}; Is the webpack-subresource-integrity
plugin installed and enabled?`);
}
sriStats[baseName] = asset.integrity;
});

fs.writeFileSync(this.saveAs, JSON.stringify(sriStats));
} catch (error) {
stats.compilation.errors.push(error);
}
});
}
}
116 changes: 116 additions & 0 deletions tests/server/TestSriDataPlugin.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
import fs from 'fs';
import path from 'path';

import { assert } from 'chai';
import webpack from 'webpack';
import SriPlugin from 'webpack-subresource-integrity';
import tmp from 'tmp';

import SriDataPlugin from 'core/server/sriDataPlugin';

describe('SriDataPlugin', () => {
let distDir;
let srcDir;
let tempDir;

beforeEach(() => {
tempDir = tmp.dirSync({ unsafeCleanup: true });
srcDir = path.join(tempDir.name, 'src');
distDir = path.join(tempDir.name, 'dist');

fs.mkdirSync(srcDir);
fs.mkdirSync(distDir);

fs.writeFileSync(path.join(srcDir, 'app.js'), '// some JS code');
});

afterEach(() => {
tempDir.removeCallback();
});

function compile({
entry = {}, includeSriPlugin = true,
} = {}) {
const sriFile = path.join(distDir, 'sri.json');

const plugins = [];
if (includeSriPlugin) {
plugins.push(new SriPlugin({ hashFuncNames: ['sha512'] }));
}
plugins.push(new SriDataPlugin({ saveAs: sriFile }));

const compiler = webpack({
entry: {
app: path.join(srcDir, 'app'),
...entry,
},
output: {
crossOriginLoading: 'anonymous',
path: path.join(tempDir.name, 'dist'),
filename: '[name].js',
chunkFilename: '[name].js',
},
plugins,
});

return new Promise((resolve, reject) => compiler.run((error, stats) => {
if (error) {
return reject(error);
}
if (stats.compilation.errors && stats.compilation.errors.length) {
return reject(new Error(
`Webpack errors: ${stats.compilation.errors.join('; ')}`
));
}
return resolve({
stats,
sriData: JSON.parse(fs.readFileSync(sriFile)),
});
}));
}

it('handles a single asset file', () => {
return compile({
entry: {
app: path.join(srcDir, 'app'),
},
})
.then(({ sriData }) => {
assert.match(sriData['app.js'], /^sha512-.*/);
});
});

it('handles multiple asset files', () => {
return compile({
entry: {
app: path.join(srcDir, 'app'),
app2: path.join(srcDir, 'app'),
},
})
.then(({ sriData }) => {
assert.match(sriData['app.js'], /^sha512-.*/);
assert.match(sriData['app2.js'], /^sha512-.*/);
});
});

it('requires the SriPlugin', () => {
return compile({
entry: {
app: path.join(srcDir, 'app'),
},
includeSriPlugin: false,
})
.then(
() => assert.fail(null, null, 'Unexpected success'),
(error) => {
assert.match(error.message,
/The integrity property is falsey for asset app\.js/);
}
);
});

it('requires a saveAs parameter', () => {
assert.throws(
() => new SriDataPlugin(), /saveAs parameter cannot be empty/);
});
});
Loading