From e35ffe3fb300faf3e90914ac14543a8976972608 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Wed, 6 Dec 2017 14:32:25 -0500 Subject: [PATCH 1/3] feat(@angular/cli): optimize stylesheets after full bundling --- package.json | 2 +- .../cli/models/webpack-configs/styles.ts | 20 +--- packages/@angular/cli/package.json | 2 +- .../cli/plugins/cleancss-webpack-plugin.ts | 111 ++++++++++++++++++ packages/@angular/cli/plugins/webpack.ts | 1 + packages/@angular/cli/tasks/eject.ts | 2 +- 6 files changed, 121 insertions(+), 17 deletions(-) create mode 100644 packages/@angular/cli/plugins/cleancss-webpack-plugin.ts diff --git a/package.json b/package.json index cbbfa3263239..88e5e6ffbb25 100644 --- a/package.json +++ b/package.json @@ -47,11 +47,11 @@ "autoprefixer": "^6.5.3", "chalk": "~2.2.0", "circular-dependency-plugin": "^4.2.1", + "clean-css": "^4.1.9", "common-tags": "^1.3.1", "copy-webpack-plugin": "^4.1.1", "core-object": "^3.1.0", "css-loader": "^0.28.1", - "cssnano": "^3.10.0", "denodeify": "^1.2.1", "ember-cli-string-utils": "^1.0.0", "enhanced-resolve": "^3.4.1", diff --git a/packages/@angular/cli/models/webpack-configs/styles.ts b/packages/@angular/cli/models/webpack-configs/styles.ts index 723cc9902e63..23d4d67e1ef1 100644 --- a/packages/@angular/cli/models/webpack-configs/styles.ts +++ b/packages/@angular/cli/models/webpack-configs/styles.ts @@ -6,8 +6,8 @@ import { import { extraEntryParser, getOutputHashFormat } from './utils'; import { WebpackConfigOptions } from '../webpack-config'; import { pluginArgs, postcssArgs } from '../../tasks/eject'; +import { CleanCssWebpackPlugin } from '../../plugins/cleancss-webpack-plugin'; -const cssnano = require('cssnano'); const postcssUrl = require('postcss-url'); const autoprefixer = require('autoprefixer'); const ExtractTextPlugin = require('extract-text-webpack-plugin'); @@ -45,15 +45,6 @@ export function getStylesConfig(wco: WebpackConfigOptions) { const deployUrl = wco.buildOptions.deployUrl || ''; const postcssPluginCreator = function() { - // safe settings based on: https://github.com/ben-eb/cssnano/issues/358#issuecomment-283696193 - const importantCommentRe = /@preserve|@licen[cs]e|[@#]\s*source(?:Mapping)?URL|^!/i; - const minimizeOptions = { - autoprefixer: false, // full pass with autoprefixer is run separately - safe: true, - mergeLonghand: false, // version 3+ should be safe; cssnano currently uses 2.x - discardComments : { remove: (comment: string) => !importantCommentRe.test(comment) } - }; - return [ postcssUrl([ { @@ -84,15 +75,12 @@ export function getStylesConfig(wco: WebpackConfigOptions) { ]), autoprefixer(), customProperties({ preserve: true }) - ].concat( - minimizeCss ? [cssnano(minimizeOptions)] : [] - ); + ]; }; (postcssPluginCreator as any)[postcssArgs] = { variableImports: { 'autoprefixer': 'autoprefixer', 'postcss-url': 'postcssUrl', - 'cssnano': 'cssnano', 'postcss-custom-properties': 'customProperties' }, variables: { minimizeCss, baseHref, deployUrl } @@ -222,6 +210,10 @@ export function getStylesConfig(wco: WebpackConfigOptions) { extraPlugins.push(new SuppressExtractedTextChunksWebpackPlugin()); } + if (minimizeCss) { + extraPlugins.push(new CleanCssWebpackPlugin({ sourceMap: cssSourceMap })); + } + return { entry: entryPoints, module: { rules }, diff --git a/packages/@angular/cli/package.json b/packages/@angular/cli/package.json index 85d9e349d806..2b35a0ae47de 100644 --- a/packages/@angular/cli/package.json +++ b/packages/@angular/cli/package.json @@ -35,11 +35,11 @@ "autoprefixer": "^6.5.3", "chalk": "~2.2.0", "circular-dependency-plugin": "^4.2.1", + "clean-css": "^4.1.9", "common-tags": "^1.3.1", "copy-webpack-plugin": "^4.1.1", "core-object": "^3.1.0", "css-loader": "^0.28.1", - "cssnano": "^3.10.0", "denodeify": "^1.2.1", "ember-cli-string-utils": "^1.0.0", "exports-loader": "^0.6.3", diff --git a/packages/@angular/cli/plugins/cleancss-webpack-plugin.ts b/packages/@angular/cli/plugins/cleancss-webpack-plugin.ts new file mode 100644 index 000000000000..2b469c0264ba --- /dev/null +++ b/packages/@angular/cli/plugins/cleancss-webpack-plugin.ts @@ -0,0 +1,111 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * 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 { Compiler } from 'webpack'; +import { RawSource, SourceMapSource } from 'webpack-sources'; + +const CleanCSS = require('clean-css'); + +interface Chunk { + files: string[]; +} + +export interface CleanCssWebpackPluginOptions { + sourceMap: boolean; +} + +export class CleanCssWebpackPlugin { + + constructor(private options: Partial = {}) {} + + apply(compiler: Compiler): void { + compiler.plugin('compilation', (compilation: any) => { + compilation.plugin('optimize-chunk-assets', + (chunks: Array, callback: (err?: Error) => void) => { + + const cleancss = new CleanCSS({ + compatibility: 'ie9', + level: 2, + inline: false, + returnPromise: true, + sourceMap: this.options.sourceMap, + }); + + const files: string[] = [...compilation.additionalChunkAssets]; + + chunks.forEach(chunk => { + if (chunk.files && chunk.files.length > 0) { + files.push(...chunk.files); + } + }); + + const actions = files + .filter(file => file.endsWith('.css')) + .map(file => { + const asset = compilation.assets[file]; + if (!asset) { + return Promise.resolve(); + } + + let content: string; + let map: any; + if (asset.sourceAndMap) { + const sourceAndMap = asset.sourceAndMap(); + content = sourceAndMap.source; + map = sourceAndMap.map; + } else { + content = asset.source(); + } + + if (content.length === 0) { + return Promise.resolve(); + } + + return Promise.resolve() + .then(() => cleancss.minify(content, map)) + .then((output: any) => { + 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; + } + + let newSource; + if (output.sourceMap) { + newSource = new SourceMapSource( + output.styles, + file, + output.sourceMap.toString(), + content, + map, + ); + } else { + newSource = new RawSource(output.styles); + } + + compilation.assets[file] = newSource; + }); + }); + + Promise.all(actions) + .then(() => callback()) + .catch(err => callback(err)); + }); + }); + } +} diff --git a/packages/@angular/cli/plugins/webpack.ts b/packages/@angular/cli/plugins/webpack.ts index e261571f7cc8..f590c5030412 100644 --- a/packages/@angular/cli/plugins/webpack.ts +++ b/packages/@angular/cli/plugins/webpack.ts @@ -1,5 +1,6 @@ // Exports the webpack plugins we use internally. export { BaseHrefWebpackPlugin } from '../lib/base-href-webpack/base-href-webpack-plugin'; +export { CleanCssWebpackPlugin, CleanCssWebpackPluginOptions } from './cleancss-webpack-plugin'; export { GlobCopyWebpackPlugin, GlobCopyWebpackPluginOptions } from './glob-copy-webpack-plugin'; export { NamedLazyChunksWebpackPlugin } from './named-lazy-chunks-webpack-plugin'; export { ScriptsWebpackPlugin, ScriptsWebpackPluginOptions } from './scripts-webpack-plugin'; diff --git a/packages/@angular/cli/tasks/eject.ts b/packages/@angular/cli/tasks/eject.ts index 9928b47a76ef..c7a920d76512 100644 --- a/packages/@angular/cli/tasks/eject.ts +++ b/packages/@angular/cli/tasks/eject.ts @@ -190,6 +190,7 @@ class JsonWebpackSerializer { this._addImport('webpack.optimize', 'ModuleConcatenationPlugin'); break; case angularCliPlugins.BaseHrefWebpackPlugin: + case angularCliPlugins.CleanCssWebpackPlugin: case angularCliPlugins.NamedLazyChunksWebpackPlugin: case angularCliPlugins.ScriptsWebpackPlugin: case angularCliPlugins.SuppressExtractedTextChunksWebpackPlugin: @@ -565,7 +566,6 @@ export default Task.extend({ 'webpack', 'autoprefixer', 'css-loader', - 'cssnano', 'exports-loader', 'file-loader', 'html-webpack-plugin', From 3a30e42370fabfe0067e6ef8f8ab56e137609161 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Fri, 8 Dec 2017 13:45:03 -0500 Subject: [PATCH 2/3] refactor(@ngtools/webpack): restructure webpack resource loader --- packages/@ngtools/webpack/package.json | 3 +- .../@ngtools/webpack/src/resource_loader.ts | 63 +++++++++---------- 2 files changed, 31 insertions(+), 35 deletions(-) diff --git a/packages/@ngtools/webpack/package.json b/packages/@ngtools/webpack/package.json index 45c65f3d37b6..326d0eae042f 100644 --- a/packages/@ngtools/webpack/package.json +++ b/packages/@ngtools/webpack/package.json @@ -31,7 +31,8 @@ "enhanced-resolve": "^3.1.0", "magic-string": "^0.22.3", "semver": "^5.3.0", - "source-map": "^0.5.6" + "source-map": "^0.5.6", + "webpack-sources": "^1.1.0" }, "peerDependencies": { "webpack": "^2.2.0 || ^3.0.0" diff --git a/packages/@ngtools/webpack/src/resource_loader.ts b/packages/@ngtools/webpack/src/resource_loader.ts index e914c654e6c9..62c60de414b7 100644 --- a/packages/@ngtools/webpack/src/resource_loader.ts +++ b/packages/@ngtools/webpack/src/resource_loader.ts @@ -1,5 +1,6 @@ import * as vm from 'vm'; import * as path from 'path'; +import { RawSource } from 'webpack-sources'; const NodeTemplatePlugin = require('webpack/lib/node/NodeTemplatePlugin'); const NodeTargetPlugin = require('webpack/lib/node/NodeTargetPlugin'); @@ -53,12 +54,7 @@ export class WebpackResourceLoader { new LoaderTargetPlugin('node') ); - // Store the result of the parent compilation before we start the child compilation - let assetsBeforeCompilation = Object.assign( - {}, - this._parentCompilation.assets[outputOptions.filename] - ); - + // NOTE: This is not needed with webpack 3.6+ // Fix for "Uncaught TypeError: __webpack_require__(...) is not a function" // Hot module replacement requires that every child compiler has its own // cache. @see https://github.com/ampedandwired/html-webpack-plugin/pull/179 @@ -71,9 +67,25 @@ export class WebpackResourceLoader { } }); + childCompiler.plugin('this-compilation', (compilation: any) => { + compilation.plugin('additional-assets', (callback: (err?: Error) => void) => { + const asset = compilation.assets[filePath]; + if (asset) { + this._evaluate({ outputName: filePath, source: asset.source() }) + .then(output => { + compilation.assets[filePath] = new RawSource(output); + callback(); + }) + .catch(err => callback(err)); + } else { + callback(); + } + }); + }); + // Compile and return a promise return new Promise((resolve, reject) => { - childCompiler.runAsChild((err: Error, entries: any[], childCompilation: any) => { + childCompiler.compile((err: Error, childCompilation: any) => { // Resolve / reject the promise if (childCompilation && childCompilation.errors && childCompilation.errors.length) { const errorDetails = childCompilation.errors.map(function (error: any) { @@ -83,47 +95,30 @@ export class WebpackResourceLoader { } else if (err) { reject(err); } else { - // Replace [hash] placeholders in filename - const outputName = this._parentCompilation.mainTemplate.applyPluginsWaterfall( - 'asset-path', outputOptions.filename, { - hash: childCompilation.hash, - chunk: entries[0] - }); - - // Restore the parent compilation to the state like it was before the child compilation. - Object.keys(childCompilation.assets).forEach((fileName) => { - // If it wasn't there and it's a source file (absolute path) - delete it. - if (assetsBeforeCompilation[fileName] === undefined && path.isAbsolute(fileName)) { - delete this._parentCompilation.assets[fileName]; - } else { - // Otherwise, add it to the parent compilation. - this._parentCompilation.assets[fileName] = childCompilation.assets[fileName]; + Object.keys(childCompilation.assets).forEach(assetName => { + if (assetName !== filePath && this._parentCompilation.assets[assetName] == undefined) { + this._parentCompilation.assets[assetName] = childCompilation.assets[assetName]; } }); // Save the dependencies for this resource. - this._resourceDependencies.set(outputName, childCompilation.fileDependencies); + this._resourceDependencies.set(filePath, childCompilation.fileDependencies); resolve({ // Output name. - outputName, + outputName: filePath, // Compiled code. - source: childCompilation.assets[outputName].source() + source: childCompilation.assets[filePath].source() }); } }); }); } - private _evaluate(output: CompilationOutput): Promise { + private _evaluate({ outputName, source }: CompilationOutput): Promise { try { - const outputName = output.outputName; - const vmContext = vm.createContext(Object.assign({ require: require }, global)); - const vmScript = new vm.Script(output.source, { filename: outputName }); - - // Evaluate code and cast to string - let evaluatedSource: string; - evaluatedSource = vmScript.runInContext(vmContext); + // Evaluate code + const evaluatedSource = vm.runInNewContext(source, undefined, { filename: outputName }); if (typeof evaluatedSource == 'string') { return Promise.resolve(evaluatedSource); @@ -137,6 +132,6 @@ export class WebpackResourceLoader { get(filePath: string): Promise { return this._compile(filePath) - .then((result: CompilationOutput) => this._evaluate(result)); + .then((result: CompilationOutput) => result.source); } } From 13547f08468c34ad32e6c58df44abf6be0fc6d9e Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Wed, 20 Dec 2017 10:55:09 -0500 Subject: [PATCH 3/3] fix(@ngtools/webpack): avoid caching binary resources as UTF8 --- .../@ngtools/webpack/src/compiler_host.ts | 20 ++-------------- .../@ngtools/webpack/src/resource_loader.ts | 23 ++++++++++++++----- 2 files changed, 19 insertions(+), 24 deletions(-) diff --git a/packages/@ngtools/webpack/src/compiler_host.ts b/packages/@ngtools/webpack/src/compiler_host.ts index 33cc54fe46d6..d53ba4a110bb 100644 --- a/packages/@ngtools/webpack/src/compiler_host.ts +++ b/packages/@ngtools/webpack/src/compiler_host.ts @@ -97,7 +97,6 @@ export class WebpackCompilerHost implements ts.CompilerHost { private _delegate: ts.CompilerHost; private _files: {[path: string]: VirtualFileStats | null} = Object.create(null); private _directories: {[path: string]: VirtualDirStats | null} = Object.create(null); - private _cachedResources: {[path: string]: string | undefined} = Object.create(null); private _changedFiles: {[path: string]: boolean} = Object.create(null); private _changedDirs: {[path: string]: boolean} = Object.create(null); @@ -174,8 +173,8 @@ export class WebpackCompilerHost implements ts.CompilerHost { fileName = this.resolve(fileName); if (fileName in this._files) { this._files[fileName] = null; - this._changedFiles[fileName] = true; } + this._changedFiles[fileName] = true; } fileExists(fileName: string, delegate = true): boolean { @@ -299,22 +298,7 @@ export class WebpackCompilerHost implements ts.CompilerHost { if (this._resourceLoader) { // These paths are meant to be used by the loader so we must denormalize them. const denormalizedFileName = this.denormalizePath(fileName); - const resourceDeps = this._resourceLoader.getResourceDependencies(denormalizedFileName); - - if (this._cachedResources[fileName] === undefined - || resourceDeps.some((dep) => this._changedFiles[this.resolve(dep)])) { - return this._resourceLoader.get(denormalizedFileName) - .then((resource) => { - // Add resource dependencies to the compiler host file list. - // This way we can check the changed files list to determine whether to use cache. - this._resourceLoader.getResourceDependencies(denormalizedFileName) - .forEach((dep) => this.readFile(dep)); - this._cachedResources[fileName] = resource; - return resource; - }); - } else { - return this._cachedResources[fileName]; - } + return this._resourceLoader.get(denormalizedFileName); } else { return this.readFile(fileName); } diff --git a/packages/@ngtools/webpack/src/resource_loader.ts b/packages/@ngtools/webpack/src/resource_loader.ts index 62c60de414b7..cce18e26fa77 100644 --- a/packages/@ngtools/webpack/src/resource_loader.ts +++ b/packages/@ngtools/webpack/src/resource_loader.ts @@ -18,6 +18,7 @@ export class WebpackResourceLoader { private _context: string; private _uniqueId = 0; private _resourceDependencies = new Map(); + private _cachedResources = new Map(); constructor() {} @@ -69,6 +70,11 @@ export class WebpackResourceLoader { childCompiler.plugin('this-compilation', (compilation: any) => { compilation.plugin('additional-assets', (callback: (err?: Error) => void) => { + if (this._cachedResources.has(compilation.fullHash)) { + callback(); + return; + } + const asset = compilation.assets[filePath]; if (asset) { this._evaluate({ outputName: filePath, source: asset.source() }) @@ -104,12 +110,17 @@ export class WebpackResourceLoader { // Save the dependencies for this resource. this._resourceDependencies.set(filePath, childCompilation.fileDependencies); - resolve({ - // Output name. - outputName: filePath, - // Compiled code. - source: childCompilation.assets[filePath].source() - }); + const compilationHash = childCompilation.fullHash; + if (this._cachedResources.has(compilationHash)) { + resolve({ + outputName: filePath, + source: this._cachedResources.get(compilationHash), + }); + } else { + const source = childCompilation.assets[filePath].source(); + this._cachedResources.set(compilationHash, source); + resolve({ outputName: filePath, source }); + } } }); });