-
Notifications
You must be signed in to change notification settings - Fork 12k
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
feat(build): add lazy styles/scripts #3402
Changes from all commits
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 |
---|---|---|
@@ -1,50 +1,18 @@ | ||
const path = require('path'); | ||
|
||
/** | ||
* Enumerate loaders and their dependencies from this file to let the dependency validator | ||
* know they are used. | ||
* | ||
* require('style-loader') | ||
* require('css-loader') | ||
* require('stylus-loader') | ||
* require('less-loader') | ||
* require('sass-loader') | ||
*/ | ||
const ExtractTextPlugin = require('extract-text-webpack-plugin'); | ||
|
||
export const getWebpackDevConfigPartial = function(projectRoot: string, appConfig: any) { | ||
const appRoot = path.resolve(projectRoot, appConfig.root); | ||
const styles = appConfig.styles | ||
? appConfig.styles.map((style: string) => path.resolve(appRoot, style)) | ||
: []; | ||
const cssLoaders = ['style-loader', 'css-loader?sourcemap', 'postcss-loader']; | ||
|
||
return { | ||
output: { | ||
path: path.resolve(projectRoot, appConfig.outDir), | ||
filename: '[name].bundle.js', | ||
sourceMapFilename: '[name].bundle.map', | ||
chunkFilename: '[id].chunk.js' | ||
}, | ||
module: { | ||
rules: [ | ||
// outside of main, load it via style-loader for development builds | ||
{ | ||
include: styles, | ||
test: /\.css$/, | ||
loaders: cssLoaders | ||
}, { | ||
include: styles, | ||
test: /\.styl$/, | ||
loaders: [...cssLoaders, 'stylus-loader?sourcemap'] | ||
}, { | ||
include: styles, | ||
test: /\.less$/, | ||
loaders: [...cssLoaders, 'less-loader?sourcemap'] | ||
}, { | ||
include: styles, | ||
test: /\.scss$|\.sass$/, | ||
loaders: [...cssLoaders, 'sass-loader?sourcemap'] | ||
}, | ||
] | ||
} | ||
plugins: [ | ||
new ExtractTextPlugin({filename: '[name].bundle.css'}) | ||
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 think the javascript bundle is needed for HMR support. 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. There seems to be ways of using it with extracted css: webpack-contrib/extract-text-webpack-plugin#30 (comment) I also want to note that CSS is only extracted for global styles. 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. OK. Always extracting is definitely cleaner just didn't know about the HMR workaround. |
||
] | ||
}; | ||
}; |
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.
AOT removes the need for
eval
but script-loader brings it back. Probably not something that should be addressed in this PR but I think it's important to note.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.
We always had
script-loader
for global scripts though, I merely added it conditionally instead of always having it.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.
I agree; no current action is needed. I was just thinking maybe a future TODO or some additional future documentation (post-1.0) might be useful (probably on CSP in general).