Skip to content

fix(build): Use static files for styles #2646

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

Closed
wants to merge 7 commits into from
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
"exit": "^0.1.2",
"exports-loader": "^0.6.3",
"expose-loader": "^0.7.1",
"extract-text-webpack-plugin": "^2.0.0-beta.4",
"file-loader": "^0.8.5",
"fs-extra": "^0.30.0",
"fs.realpath": "^1.0.0",
Expand Down
18 changes: 0 additions & 18 deletions packages/angular-cli/models/webpack-build-common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,24 +71,6 @@ export function getWebpackCommonConfig(
loaders: ['raw-loader', 'postcss-loader', 'sass-loader']
},

// outside of main, load it via style-loader
       {
include: styles,
test: /\.css$/,
loaders: ['style-loader', 'css-loader', 'postcss-loader']
}, {
include: styles,
test: /\.styl$/,
loaders: ['style-loader', 'css-loader', 'postcss-loader', 'stylus-loader']
}, {
include: styles,
test: /\.less$/,
loaders: ['style-loader', 'css-loader', 'postcss-loader', 'less-loader']
}, {
include: styles,
test: /\.scss$|\.sass$/,
loaders: ['style-loader', 'css-loader', 'postcss-loader', 'sass-loader']
},

// load global scripts using script-loader
{ include: scripts, test: /\.js$/, loader: 'script-loader' },
Expand Down
27 changes: 27 additions & 0 deletions packages/angular-cli/models/webpack-build-development.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,40 @@
const path = require('path');

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 {
devtool: 'cheap-module-source-map',
output: {
path: path.resolve(projectRoot, appConfig.outDir),
filename: '[name].bundle.js',
sourceMapFilename: '[name].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']
},
]
}
};
};
29 changes: 29 additions & 0 deletions packages/angular-cli/models/webpack-build-production.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as path from 'path';
const WebpackMd5Hash = require('webpack-md5-hash');
const CompressionPlugin = require('compression-webpack-plugin');
import * as webpack from 'webpack';
const ExtractTextPlugin = require('extract-text-webpack-plugin');

declare module 'webpack' {
export interface LoaderOptionsPlugin {}
Expand All @@ -14,6 +15,11 @@ declare module 'webpack' {
}

export const getWebpackProdConfigPartial = 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 = ['css-loader?sourcemap&minimize', 'postcss-loader'];
return {
devtool: 'source-map',
output: {
Expand All @@ -22,7 +28,30 @@ export const getWebpackProdConfigPartial = function(projectRoot: string, appConf
sourceMapFilename: '[name].[chunkhash].bundle.map',
chunkFilename: '[id].[chunkhash].chunk.js'
},
module: {
rules: [
// outside of main, load it via extract-text-plugin for production builds
       {
include: styles,
test: /\.css$/,
loaders: ExtractTextPlugin.extract(cssLoaders)
}, {
include: styles,
test: /\.styl$/,
loaders: ExtractTextPlugin.extract([...cssLoaders, 'stylus-loader?sourcemap'])
}, {
include: styles,
test: /\.less$/,
loaders: ExtractTextPlugin.extract([...cssLoaders, 'less-loader?sourcemap'])
}, {
include: styles,
test: /\.scss$|\.sass$/,
loaders: ExtractTextPlugin.extract([...cssLoaders, 'sass-loader?sourcemap'])
},
]
},
plugins: [
new ExtractTextPlugin('[name].[contenthash].bundle.css'),
new WebpackMd5Hash(),
new webpack.DefinePlugin({
'process.env.NODE_ENV': JSON.stringify('production')
Expand Down
1 change: 1 addition & 0 deletions packages/angular-cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
"exit": "^0.1.2",
"exports-loader": "^0.6.3",
"expose-loader": "^0.7.1",
"extract-text-webpack-plugin": "^2.0.0-beta.4",
"file-loader": "^0.8.5",
"fs-extra": "^0.30.0",
"fs.realpath": "^1.0.0",
Expand Down
1 change: 1 addition & 0 deletions tests/e2e/tests/build/prod-build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export default function() {
.then(() => expectFileToExist(join(process.cwd(), 'dist')))
// Check for cache busting hash script src
.then(() => expectFileToMatch('dist/index.html', /main\.[0-9a-f]{20}\.bundle\.js/))
.then(() => expectFileToMatch('dist/index.html', /styles\.[0-9a-f]{32}\.bundle\.css/))

// Check that the process didn't change local files.
.then(() => expectGitToBeClean())
Expand Down
38 changes: 36 additions & 2 deletions tests/e2e/tests/build/styles/css.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,53 @@
import * as glob from 'glob';

import {writeMultipleFiles, expectFileToMatch} from '../../../utils/fs';
import {ng} from '../../../utils/process';
import {updateJsonFile} from '../../../utils/project';


export default function() {
return writeMultipleFiles({
'src/styles.css': `
@import './imported-styles.css';

body { background-color: blue; }
`,
'src/imported-styles.css': `
p { background-color: red; }
`,
'src/styles.less': `
.outer {
.inner {
background: #fff;
}
}
`,
'src/styles.scss': `
.upper {
.lower {
background: #def;
}
}
`
})
.then(() => updateJsonFile('angular-cli.json', configJson => {
const app = configJson['apps'][0];
app['styles'].push('styles.less');
app['styles'].push('styles.scss');
}))
.then(() => ng('build'))
.then(() => expectFileToMatch('dist/styles.bundle.js', 'body { background-color: blue; }'))
.then(() => expectFileToMatch('dist/styles.bundle.js', 'p { background-color: red; }'));
.then(() => expectFileToMatch('dist/styles.bundle.js', 'p { background-color: red; }'))
.then(() => expectFileToMatch('dist/styles.bundle.js', /.outer.*.inner.*background:\s*#[fF]+/))
.then(() => expectFileToMatch('dist/styles.bundle.js', /.upper.*.lower.*background.*#def/))

.then(() => ng('build', '--prod'))
.then(() => new Promise<string>(() =>
glob.sync('dist/styles.*.bundle.css').find(file => !!file)))
Copy link
Contributor

Choose a reason for hiding this comment

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

You never resolve this promise, causing tests to stop early. This test is causing our e2e to not continue.

Also, no need to return a promise, just return the glob.sync.

.then((styles) =>
expectFileToMatch(styles, 'body { background-color: blue; }')
.then(() => expectFileToMatch(styles, 'p { background-color: red; }')
.then(() => expectFileToMatch(styles, /.outer.*.inner.*background:\s*#[fF]+/))
.then(() => expectFileToMatch(styles, /.upper.*.lower.*background.*#def/)))
);
}