Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): change css optimizer from clean-c…
Browse files Browse the repository at this point in the history
…ss with cssnano

Closes #16123 and closes #13854
  • Loading branch information
alan-agius4 authored and mgechev committed Jan 13, 2020
1 parent db6dd3d commit df927d0
Show file tree
Hide file tree
Showing 8 changed files with 776 additions and 112 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@
"@types/babel__core": "7.1.3",
"@types/browserslist": "^4.4.0",
"@types/caniuse-lite": "^1.0.0",
"@types/clean-css": "^4.2.1",
"@types/copy-webpack-plugin": "^4.4.1",
"@types/cssnano": "^4.0.0",
"@types/express": "^4.16.0",
"@types/fast-json-stable-stringify": "^2.0.0",
"@types/find-cache-dir": "^2.0.0",
Expand Down
4 changes: 2 additions & 2 deletions packages/angular_devkit/build_angular/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
"babel-loader": "8.0.6",
"browserslist": "4.8.3",
"cacache": "13.0.1",
"caniuse-lite": "1.0.30001019",
"caniuse-lite": "1.0.30001020",
"cssnano": "4.1.10",
"circular-dependency-plugin": "5.2.0",
"clean-css": "4.2.1",
"coverage-istanbul-loader": "2.0.3",
"copy-webpack-plugin": "5.1.1",
"core-js": "3.6.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ import { BuildBrowserFeatures } from '../../../utils';
import { findCachePath } from '../../../utils/cache-path';
import { cachingDisabled, manglingDisabled } from '../../../utils/environment-options';
import { BundleBudgetPlugin } from '../../plugins/bundle-budget';
import { CleanCssWebpackPlugin } from '../../plugins/cleancss-webpack-plugin';
import { NamedLazyChunksPlugin } from '../../plugins/named-chunks-plugin';
import { OptimizeCssWebpackPlugin } from '../../plugins/optimize-css-webpack-plugin';
import { ScriptsWebpackPlugin } from '../../plugins/scripts-webpack-plugin';
import { WebpackRollupLoader } from '../../plugins/webpack';
import { findAllNodeModules, findUp } from '../../utilities/find-up';
Expand Down Expand Up @@ -363,7 +363,7 @@ export function getCommonConfig(wco: WebpackConfigOptions): Configuration {
const extraMinimizers = [];
if (stylesOptimization) {
extraMinimizers.push(
new CleanCssWebpackPlugin({
new OptimizeCssWebpackPlugin({
sourceMap: stylesSourceMap,
// component styles retain their original file name
test: file => /\.(?:css|scss|sass|less|styl)$/.test(file),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import * as CleanCSS from 'clean-css';
import * as cssNano from 'cssnano';
import { ProcessOptions, Result } from 'postcss';
import { Compiler, compilation } from 'webpack';
import { RawSource, Source, SourceMapSource } from 'webpack-sources';

export interface CleanCssWebpackPluginOptions {
export interface OptimizeCssWebpackPluginOptions {
sourceMap: boolean;
test: (file: string) => boolean;
}
Expand All @@ -22,19 +23,19 @@ function hook(
) => Promise<void | void[]>,
) {
compiler.hooks.compilation.tap(
'cleancss-webpack-plugin',
'optimize-css-webpack-plugin',
(compilation: compilation.Compilation) => {
compilation.hooks.optimizeChunkAssets.tapPromise('cleancss-webpack-plugin', chunks =>
compilation.hooks.optimizeChunkAssets.tapPromise('optimize-css-webpack-plugin', chunks =>
action(compilation, chunks),
);
},
);
}

export class CleanCssWebpackPlugin {
private readonly _options: CleanCssWebpackPluginOptions;
export class OptimizeCssWebpackPlugin {
private readonly _options: OptimizeCssWebpackPluginOptions;

constructor(options: Partial<CleanCssWebpackPluginOptions>) {
constructor(options: Partial<OptimizeCssWebpackPluginOptions>) {
this._options = {
sourceMap: false,
test: file => file.endsWith('.css'),
Expand All @@ -44,21 +45,6 @@ export class CleanCssWebpackPlugin {

apply(compiler: Compiler): void {
hook(compiler, (compilation: compilation.Compilation, chunks: compilation.Chunk[]) => {
const cleancss = new CleanCSS({
compatibility: 'ie9',
level: {
2: {
skipProperties: [
'transition', // Fixes #12408
'font', // Fixes #9648
],
},
},
inline: false,
returnPromise: true,
sourceMap: this._options.sourceMap,
});

const files: string[] = [...compilation.additionalChunkAssets];

chunks.forEach(chunk => {
Expand Down Expand Up @@ -90,37 +76,40 @@ export class CleanCssWebpackPlugin {
return;
}

const output = await cleancss.minify(content, map);

let hasWarnings = false;
if (output.warnings && output.warnings.length > 0) {
compilation.warnings.push(...output.warnings);
hasWarnings = true;
}

if (output.errors && output.errors.length > 0) {
output.errors.forEach((error: string) => compilation.errors.push(new Error(error)));

return;
}

// generally means invalid syntax so bail
if (hasWarnings && output.stats.minifiedSize === 0) {
return;
const cssNanoOptions: cssNano.CssNanoOptions = {
preset: 'default',
};

const postCssOptions: ProcessOptions = {
from: file,
map: map && { annotation: false, prev: map },
};

const output = await new Promise<Result>((resolve, reject) => {
// the last parameter is not in the typings
// tslint:disable-next-line: no-any
(cssNano.process as any)(content, postCssOptions, cssNanoOptions)
.then(resolve)
.catch(reject);
});

const warnings = output.warnings();
if (warnings.length) {
compilation.warnings.push(...warnings.map(({ text }) => text));
}

let newSource;
if (output.sourceMap) {
if (output.map) {
newSource = new SourceMapSource(
output.styles,
output.css,
file,
// tslint:disable-next-line: no-any
output.sourceMap.toString() as any,
output.map.toString() as any,
content,
map,
);
} else {
newSource = new RawSource(output.styles);
newSource = new RawSource(output.css);
}

compilation.assets[file] = newSource;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

// Exports the webpack plugins we use internally.
export { AnyComponentStyleBudgetChecker } from './any-component-style-budget-checker';
export { CleanCssWebpackPlugin, CleanCssWebpackPluginOptions } from './cleancss-webpack-plugin';
export { OptimizeCssWebpackPlugin, OptimizeCssWebpackPluginOptions } from './optimize-css-webpack-plugin';
export { BundleBudgetPlugin, BundleBudgetPluginOptions } from './bundle-budget';
export { ScriptsWebpackPlugin, ScriptsWebpackPluginOptions } from './scripts-webpack-plugin';
export { SuppressExtractedTextChunksWebpackPlugin } from './suppress-entry-chunks-webpack-plugin';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,30 +40,6 @@ describe('Browser Builder AOT', () => {
await run.stop();
});

it('shows warnings for component styles', async () => {
const overrides = {
aot: true,
optimization: true,
};

host.writeMultipleFiles({
'src/app/app.component.css': `
.foo { color: white; padding: 1px; };
.buz { color: white; padding: 2px; };
`,
});

const logger = new logging.Logger('');
const logs: string[] = [];
logger.subscribe(e => logs.push(e.message));

const run = await architect.scheduleTarget(targetSpec, overrides, { logger });
const output = await run.result;
expect(output.success).toBe(true);
expect(logs.join()).toContain('WARNING in Invalid selector');
await run.stop();
});

it('shows error when component stylesheet contains SCSS syntax error', async () => {
const overrides = {
aot: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -585,23 +585,4 @@ describe('Browser Builder styles', () => {
const { output } = await browserBuild(architect, host, target, overrides);
expect(output.success).toBe(true);
});

it('supports font names with spaces', async () => {
host.writeMultipleFiles({
'src/styles.css': `
body {
font: 10px "Font Awesome";
}
`,
});

const overrides = { extractCss: true, optimization: true };
const logger = new logging.Logger('font-name-spaces');
const logs: string[] = [];
logger.subscribe(e => logs.push(e.message));

const { output } = await browserBuild(architect, host, target, overrides, { logger });
expect(output.success).toBe(true);
expect(logs.join()).not.toContain('WARNING in Invalid font values ');
});
});
Loading

0 comments on commit df927d0

Please sign in to comment.