-
Notifications
You must be signed in to change notification settings - Fork 400
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
Upgrade to webpack 2 #2380
Changes from 16 commits
2294658
631a9a2
1979568
a688738
999cf94
488aba2
84ec3ec
f323abe
142d1b7
99be0a7
c52d771
ed1308b
eff7d10
0f096fb
d093fd9
79ea4ed
0becc82
732a201
19d7e96
8ec66ce
3eb2ff6
eb720de
cec2440
5ac680f
e3f3895
ee7faab
12d0079
6291206
0f3865e
69f3e4d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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", | ||
|
@@ -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", | ||
|
@@ -285,15 +285,15 @@ | |
"semver": "^5.3.0", | ||
"shelljs": "^0.7.7", | ||
"sinon": "^2.2.0", | ||
"sri-stats-webpack-plugin": "^0.7.3", | ||
"sri-stats-webpack-plugin": "github:sopherio/sri-stats-webpack-plugin#f23de19e1d84dab50e59076e05bcafa508ba64c4", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is pretty unfortunate since the patch to make it webpack 2 compatible is just a dependency bump. More info: mikechau/sri-stats-webpack-plugin#71 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we should consider an alternative lib e.g: https://github.com/waysact/webpack-subresource-integrity (there might be others, though this says it supports both webpack1 and 2). The bit I'm not keen on is that this change relies on a fork, which in turn loads another fork. Whilst you've been able to specify a hash in the first case, you can't in the second. To fix potential integrity issue we'd need to have our own forks or, it might be easier to use something else? |
||
"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", | ||
"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" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,173 @@ | ||
/* eslint-disable import/no-extraneous-dependencies */ | ||
import config from 'config'; | ||
import ExtractTextPlugin from 'extract-text-webpack-plugin'; | ||
import webpack from 'webpack'; | ||
|
||
import { getClientConfig } from 'core/utils'; | ||
|
||
export function getRules({ babelQuery, bundleStylesWithJs = false } = {}) { | ||
let styleRules; | ||
|
||
if (bundleStylesWithJs) { | ||
// In development, we bundle styles with the JS. | ||
styleRules = [ | ||
{ | ||
test: /\.css$/, | ||
use: [ | ||
{ loader: 'style-loader' }, | ||
{ loader: 'css-loader', options: { importLoaders: 2 } }, | ||
{ loader: 'postcss-loader', options: { outputStyle: 'expanded' } }, | ||
], | ||
}, { | ||
test: /\.scss$/, | ||
use: [ | ||
{ loader: 'style-loader' }, | ||
{ loader: 'css-loader', options: { importLoaders: 2 } }, | ||
{ loader: 'postcss-loader' }, | ||
{ loader: 'sass-loader', options: { outputStyle: 'expanded' } }, | ||
], | ||
}, | ||
]; | ||
} else { | ||
// In production, we create a separate CSS bundle rather than | ||
// include styles with the JS bundle. This lets the style bundle | ||
// load in parallel. | ||
styleRules = [ | ||
{ | ||
test: /\.css$/, | ||
loader: ExtractTextPlugin.extract({ | ||
fallback: 'style-loader', | ||
use: [ | ||
{ | ||
loader: 'css-loader', | ||
options: { importLoaders: 2, sourceMap: true }, | ||
}, | ||
{ | ||
loader: 'postcss-loader', | ||
options: { | ||
outputStyle: 'expanded', | ||
sourceMap: true, | ||
sourceMapContents: true, | ||
}, | ||
}, | ||
], | ||
}), | ||
}, { | ||
test: /\.scss$/, | ||
loader: ExtractTextPlugin.extract({ | ||
fallback: 'style-loader', | ||
use: [ | ||
{ | ||
loader: 'css-loader', | ||
options: { importLoaders: 2, sourceMap: true }, | ||
}, | ||
{ loader: 'postcss-loader' }, | ||
{ | ||
loader: 'sass-loader', | ||
options: { | ||
outputStyle: 'expanded', | ||
sourceMap: true, | ||
sourceMapContents: true, | ||
}, | ||
}, | ||
], | ||
}), | ||
}, | ||
]; | ||
} | ||
|
||
return [ | ||
{ | ||
test: /\.jsx?$/, | ||
exclude: /node_modules/, | ||
loader: 'babel-loader', | ||
query: babelQuery, | ||
}, | ||
...styleRules, | ||
{ | ||
test: /\.svg$/, | ||
use: [{ loader: 'svg-url-loader', options: { limit: 10000 } }], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this have a mimetype added? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just read the docs on these loaders. I can add one here but why are we specifying mime types at all? The docs say that a mime type is inferred from file extension. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it's not needed then that's less code to maintain here 👍 |
||
}, { | ||
test: /\.jpg$/, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
use: [{ | ||
loader: 'url-loader', | ||
options: { limit: 10000, mimetype: 'image/jpeg' }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 10000 could be a |
||
}], | ||
}, { | ||
test: /\.png$/, | ||
use: [{ | ||
loader: 'url-loader', | ||
options: { limit: 10000, mimetype: 'image/png' }, | ||
}], | ||
}, { | ||
test: /\.gif/, | ||
use: [{ | ||
loader: 'url-loader', | ||
options: { limit: 10000, mimetype: 'image/gif' }, | ||
}], | ||
}, { | ||
test: /\.webm$/, | ||
use: [{ | ||
loader: 'url-loader', | ||
options: { limit: 10000, mimetype: 'video/webm' }, | ||
}], | ||
}, { | ||
test: /\.mp4$/, | ||
use: [{ | ||
loader: 'url-loader', | ||
options: { limit: 10000, mimetype: 'video/mp4' }, | ||
}], | ||
}, { | ||
test: /\.otf$/, | ||
use: [{ | ||
loader: 'url-loader', | ||
options: { limit: 10000, mimetype: 'application/font-sfnt' }, | ||
}], | ||
}, { | ||
test: /\.woff$/, | ||
use: [{ | ||
loader: 'url-loader', | ||
options: { limit: 10000, mimetype: 'application/font-woff' }, | ||
}], | ||
}, { | ||
test: /\.woff2$/, | ||
use: [{ | ||
loader: 'url-loader', | ||
options: { limit: 10000, mimetype: 'application/font-woff2' }, | ||
}], | ||
}, | ||
]; | ||
} | ||
|
||
export function getPlugins({ excludeOtherAppLocales = true } = {}) { | ||
const appName = config.get('appName'); | ||
const clientConfig = getClientConfig(config); | ||
|
||
const plugins = [ | ||
new webpack.DefinePlugin({ | ||
CLIENT_CONFIG: JSON.stringify(clientConfig), | ||
'process.env.NODE_ENV': JSON.stringify('production'), | ||
}), | ||
// Replaces server config module with the subset clientConfig object. | ||
new webpack.NormalModuleReplacementPlugin( | ||
/config$/, 'core/client/config.js'), | ||
// Substitutes client only config. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment should be updated to refer to logging. |
||
new webpack.NormalModuleReplacementPlugin( | ||
/core\/logger$/, 'core/client/logger.js'), | ||
// Use the browser's window for window. | ||
new webpack.NormalModuleReplacementPlugin( | ||
/core\/window/, 'core/browserWindow.js'), | ||
]; | ||
|
||
if (excludeOtherAppLocales) { | ||
plugins.push( | ||
// This allow us to exclude locales for other apps being built. | ||
new webpack.ContextReplacementPlugin( | ||
/locale$/, | ||
new RegExp(`^\\.\\/.*?\\/${appName}\\.js$`) | ||
), | ||
); | ||
} | ||
|
||
return plugins; | ||
} |
There was a problem hiding this comment.
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, theprocess.env.NODE_ENV
value is added toDefinePlugin
unlike before. This doesn't seem to cause any problems though.