From 71768d369dd3b9123ff90c3f66998b93fab9ace1 Mon Sep 17 00:00:00 2001 From: Alan Date: Tue, 16 Jul 2019 11:49:26 +0200 Subject: [PATCH] feat(@ngtools/webpack): show warning when large CSS are included in components MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s very easy to inadvertently import toplevel css in component styles. Since component css is standalone and self-contained, it will never be shared between components and remains as a single large bundle for each component. This in turn adds a large amount of code that must be processed and increases bundle size. Related to: TOOL-949 --- .../webpack/src/angular_compiler_plugin.ts | 38 +++++++++++++++++-- .../src/transformers/find_resources.ts | 11 +++--- .../src/transformers/find_resources_spec.ts | 9 +++-- packages/ngtools/webpack/src/utils.ts | 10 +++++ 4 files changed, 56 insertions(+), 12 deletions(-) diff --git a/packages/ngtools/webpack/src/angular_compiler_plugin.ts b/packages/ngtools/webpack/src/angular_compiler_plugin.ts index 872b47f37ca7..506665eff7de 100644 --- a/packages/ngtools/webpack/src/angular_compiler_plugin.ts +++ b/packages/ngtools/webpack/src/angular_compiler_plugin.ts @@ -71,7 +71,7 @@ import { MESSAGE_KIND, UpdateMessage, } from './type_checker_messages'; -import { flattenArray, forwardSlashPath, workaroundResolve } from './utils'; +import { flattenArray, formatSize, forwardSlashPath, workaroundResolve } from './utils'; import { VirtualFileSystemDecorator, VirtualWatchFileSystemDecorator, @@ -85,6 +85,12 @@ import { WebpackInputHost } from './webpack-input-host'; const treeKill = require('tree-kill'); +/** + * The recommended maximum size of components styles in bytes. + */ +const stylesSizeLimit = 70 * 1024; +const formattedStylesSizeLimit = formatSize(stylesSizeLimit); + export class AngularCompilerPlugin { private _options: AngularCompilerPluginOptions; @@ -131,6 +137,8 @@ export class AngularCompilerPlugin { private _mainFields: string[] = []; + private _stylesResources = new Set(); + constructor(options: AngularCompilerPluginOptions) { this._options = Object.assign({}, options); this._setupOptions(this._options); @@ -788,6 +796,24 @@ export class AngularCompilerPlugin { compiler.hooks.afterEmit.tap('angular-compiler', compilation => { // tslint:disable-next-line:no-any (compilation as any)._ngToolsWebpackPluginInstance = null; + + // CSS size checks + const stylesSizeWarning: string[] = []; + this._stylesResources.forEach(fileName => { + const styleModule = compilation.modules.find( + x => typeof x.resource === 'string' && x.resource.replace(/\.shim\.ngstyle\.js$/, '') === fileName + ); + const styleModuleSize = styleModule && styleModule.size(); + if (styleModuleSize && styleModuleSize > stylesSizeLimit) { + stylesSizeWarning.push( + `${fileName} (${formatSize(styleModuleSize)}) component style size exceeds the recommended limit (${formattedStylesSizeLimit}).`, + ); + } + }) + + if (stylesSizeWarning.length) { + compilation.warnings.push(...stylesSizeWarning); + } }); compiler.hooks.done.tap('angular-compiler', () => { this._donePromise = null; @@ -1176,16 +1202,20 @@ export class AngularCompilerPlugin { }) .filter(x => x); - const resourceImports = findResources(sourceFile) - .map(resourcePath => resolve(dirname(resolvedFileName), normalize(resourcePath))); + const { templates, styles } = findResources(sourceFile); + const templatesImports = templates.map(resourcePath => resolve(dirname(resolvedFileName), normalize(resourcePath))); + const stylesImports = styles.map(resourcePath => resolve(dirname(resolvedFileName), normalize(resourcePath))); // These paths are meant to be used by the loader so we must denormalize them. const uniqueDependencies = new Set([ ...esImports, - ...resourceImports, + ...stylesImports, + ...templatesImports, ...this.getResourceDependencies(this._compilerHost.denormalizePath(resolvedFileName)), ].map((p) => p && this._compilerHost.denormalizePath(p))); + stylesImports.forEach(f => this._stylesResources.add(this._compilerHost.denormalizePath(f))); + return [...uniqueDependencies] .filter(x => !!x) as string[]; } diff --git a/packages/ngtools/webpack/src/transformers/find_resources.ts b/packages/ngtools/webpack/src/transformers/find_resources.ts index 1158e4465fde..c697acb1b0df 100644 --- a/packages/ngtools/webpack/src/transformers/find_resources.ts +++ b/packages/ngtools/webpack/src/transformers/find_resources.ts @@ -9,9 +9,10 @@ import * as ts from 'typescript'; import { collectDeepNodes } from './ast_helpers'; import { getResourceUrl } from './replace_resources'; -export function findResources(sourceFile: ts.SourceFile): string[] { - const resources: string[] = []; +export function findResources(sourceFile: ts.SourceFile): { templates: string[]; styles: string[]} { const decorators = collectDeepNodes(sourceFile, ts.SyntaxKind.Decorator); + const templates: string[] = []; + const styles: string[] = []; for (const node of decorators) { if (!ts.isCallExpression(node.expression)) { @@ -38,7 +39,7 @@ export function findResources(sourceFile: ts.SourceFile): string[] { const url = getResourceUrl(node.initializer); if (url) { - resources.push(url); + templates.push(url); } break; @@ -51,7 +52,7 @@ export function findResources(sourceFile: ts.SourceFile): string[] { const url = getResourceUrl(node); if (url) { - resources.push(url); + styles.push(url); } return node; @@ -64,5 +65,5 @@ export function findResources(sourceFile: ts.SourceFile): string[] { ); } - return resources; + return { templates, styles }; } diff --git a/packages/ngtools/webpack/src/transformers/find_resources_spec.ts b/packages/ngtools/webpack/src/transformers/find_resources_spec.ts index 0e0cf9cbaaa8..a36a91104f2b 100644 --- a/packages/ngtools/webpack/src/transformers/find_resources_spec.ts +++ b/packages/ngtools/webpack/src/transformers/find_resources_spec.ts @@ -27,11 +27,13 @@ describe('@ngtools/webpack transformers', () => { `; const result = findResources(ts.createSourceFile('temp.ts', input, ts.ScriptTarget.ES2015)); - expect(result).toEqual([ - './app.component.html', + expect(result.styles).toEqual([ './app.component.css', './app.component.2.css', ]); + expect(result.templates).toEqual([ + './app.component.html', + ]); }); it('should not return resources if they are not in decorator', () => { @@ -43,7 +45,8 @@ describe('@ngtools/webpack transformers', () => { `; const result = findResources(ts.createSourceFile('temp.ts', input, ts.ScriptTarget.ES2015)); - expect(result).toEqual([]); + expect(result.styles).toEqual([]); + expect(result.templates).toEqual([]); }); }); }); diff --git a/packages/ngtools/webpack/src/utils.ts b/packages/ngtools/webpack/src/utils.ts index db4336ada4b2..8b5f1aced8b7 100644 --- a/packages/ngtools/webpack/src/utils.ts +++ b/packages/ngtools/webpack/src/utils.ts @@ -23,3 +23,13 @@ export function flattenArray(value: Array): T[] { export function forwardSlashPath(path: string) { return path.replace(/\\/g, '/'); } + +export function formatSize(size: number): string { + if (size <= 0) { + return '0 bytes'; + } + const abbreviations = ['bytes', 'kB', 'MB', 'GB']; + const index = Math.floor(Math.log(size) / Math.log(1024)); + + return `${+(size / Math.pow(1024, index)).toPrecision(3)} ${abbreviations[index]}`; +}