From a2342b6a75fee3181397f7a6e78642a62812640e Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Wed, 29 Aug 2018 18:27:21 +0200 Subject: [PATCH] feat(@angular-devkit/build-angular): remove inlining of assets in css (#12027) BREAKING CHANGE: Assets under 10Kib are not longer inlined in css --- .../models/webpack-configs/styles.ts | 41 +++++------- .../test/browser/styles_spec_large.ts | 66 ++---------------- .../e2e/tests/build/styles/inline-urls.ts | 67 ------------------- 3 files changed, 21 insertions(+), 153 deletions(-) delete mode 100644 tests/legacy-cli/e2e/tests/build/styles/inline-urls.ts diff --git a/packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/styles.ts b/packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/styles.ts index b8d067df51..353e2b91ff 100644 --- a/packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/styles.ts +++ b/packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/styles.ts @@ -53,8 +53,6 @@ export function getStylesConfig(wco: WebpackConfigOptions) { const extraPlugins: any[] = []; const cssSourceMap = buildOptions.sourceMap; - // Maximum resource size to inline (KiB) - const maximumInlineSize = 10; // Determine hashing format. const hashFormat = getOutputHashFormat(buildOptions.outputHashing as string); // Convert absolute resource URLs to account for base-href and deploy-url. @@ -134,18 +132,7 @@ export function getStylesConfig(wco: WebpackConfigOptions) { return `/${baseHref}/${deployUrl}/${url}`.replace(/\/\/+/g, '/'); } } - }, - { - // TODO: inline .cur if not supporting IE (use browserslist to check) - filter: (asset: PostcssUrlAsset) => { - return maximumInlineSize > 0 && !asset.hash && !asset.absolutePath.endsWith('.cur'); - }, - url: 'inline', - // NOTE: maxSize is in KB - maxSize: maximumInlineSize, - fallback: 'rebase', - }, - { url: 'rebase' }, + } ]), PostcssCliResources({ deployUrl: loader.loaders[loader.loaderIndex].options.ident == 'extracted' ? '' : deployUrl, @@ -196,7 +183,7 @@ export function getStylesConfig(wco: WebpackConfigOptions) { if (chunkNames.length > 0) { // Add plugin to remove hashes from lazy styles. - extraPlugins.push(new RemoveHashPlugin({ chunkNames, hashFormat})); + extraPlugins.push(new RemoveHashPlugin({ chunkNames, hashFormat })); } } @@ -216,7 +203,8 @@ export function getStylesConfig(wco: WebpackConfigOptions) { const baseRules: webpack.Rule[] = [ { test: /\.css$/, use: [] }, { - test: /\.scss$|\.sass$/, use: [{ + test: /\.scss$|\.sass$/, + use: [{ loader: 'sass-loader', options: { implementation: dartSass, @@ -229,7 +217,8 @@ export function getStylesConfig(wco: WebpackConfigOptions) { }] }, { - test: /\.less$/, use: [{ + test: /\.less$/, + use: [{ loader: 'less-loader', options: { sourceMap: cssSourceMap, @@ -239,7 +228,8 @@ export function getStylesConfig(wco: WebpackConfigOptions) { }] }, { - test: /\.styl$/, use: [{ + test: /\.styl$/, + use: [{ loader: 'stylus-loader', options: { sourceMap: cssSourceMap, @@ -251,7 +241,9 @@ export function getStylesConfig(wco: WebpackConfigOptions) { // load component css as raw strings const rules: webpack.Rule[] = baseRules.map(({ test, use }) => ({ - exclude: globalStylePaths, test, use: [ + exclude: globalStylePaths, + test, + use: [ { loader: 'raw-loader' }, { loader: 'postcss-loader', @@ -306,16 +298,17 @@ export function getStylesConfig(wco: WebpackConfigOptions) { } if (buildOptions.extractCss) { - // extract global css from js files into own css file extraPlugins.push( - new MiniCssExtractPlugin({ filename: `[name]${hashFormat.extract}.css` })); - // suppress empty .js files in css only entry points - extraPlugins.push(new SuppressExtractedTextChunksWebpackPlugin()); + // extract global css from js files into own css file + new MiniCssExtractPlugin({ filename: `[name]${hashFormat.extract}.css` }), + // suppress empty .js files in css only entry points + new SuppressExtractedTextChunksWebpackPlugin(), + ); } return { entry: entryPoints, module: { rules }, - plugins: [].concat(extraPlugins as any) + plugins: extraPlugins }; } diff --git a/packages/angular_devkit/build_angular/test/browser/styles_spec_large.ts b/packages/angular_devkit/build_angular/test/browser/styles_spec_large.ts index e6bb8cf866..774e1ae967 100644 --- a/packages/angular_devkit/build_angular/test/browser/styles_spec_large.ts +++ b/packages/angular_devkit/build_angular/test/browser/styles_spec_large.ts @@ -278,62 +278,6 @@ describe('Browser Builder styles', () => { }); }); - it('inlines resources', (done) => { - host.copyFile('src/spectrum.png', 'src/large.png'); - host.writeMultipleFiles({ - 'src/styles.scss': ` - h1 { background: url('./large.png'), - linear-gradient(to bottom, #0e40fa 25%, #0654f4 75%); } - h2 { background: url('./small.svg'); } - p { background: url(./small-id.svg#testID); } - `, - 'src/app/app.component.css': ` - h3 { background: url('../small.svg'); } - h4 { background: url("../large.png"); } - `, - 'src/small.svg': imgSvg, - 'src/small-id.svg': imgSvg, - }); - - const overrides = { - aot: true, - extractCss: true, - styles: [`src/styles.scss`], - }; - - runTargetSpec(host, browserTargetSpec, overrides).pipe( - tap((buildEvent) => expect(buildEvent.success).toBe(true)), - tap(() => { - const fileName = 'dist/styles.css'; - const content = virtualFs.fileBufferToString(host.scopedSync().read(normalize(fileName))); - // Large image should not be inlined, and gradient should be there. - expect(content).toMatch( - /url\(['"]?large\.png['"]?\),\s+linear-gradient\(to bottom, #0e40fa 25%, #0654f4 75%\);/); - // Small image should be inlined. - expect(content).toMatch(/url\(\\?['"]data:image\/svg\+xml/); - // Small image with param should not be inlined. - expect(content).toMatch(/url\(['"]?small-id\.svg#testID['"]?\)/); - }), - tap(() => { - const fileName = 'dist/main.js'; - const content = virtualFs.fileBufferToString(host.scopedSync().read(normalize(fileName))); - // Large image should not be inlined. - expect(content).toMatch(/url\((?:['"]|\\')?large\.png(?:['"]|\\')?\)/); - // Small image should be inlined. - expect(content).toMatch(/url\(\\?['"]data:image\/svg\+xml/); - }), - tap(() => { - expect(host.scopedSync().exists(normalize('dist/small.svg'))).toBe(false); - expect(host.scopedSync().exists(normalize('dist/large.png'))).toBe(true); - expect(host.scopedSync().exists(normalize('dist/small-id.svg'))).toBe(true); - }), - // TODO: find a way to check logger/output for warnings. - // if (stdout.match(/postcss-url: \.+: Can't read file '\.+', ignoring/)) { - // throw new Error('Expected no postcss-url file read warnings.'); - // } - ).toPromise().then(done, done.fail); - }); - it(`supports font-awesome imports`, (done) => { host.writeMultipleFiles({ 'src/styles.scss': ` @@ -407,8 +351,6 @@ describe('Browser Builder styles', () => { h3 { background: url('/assets/component-img-absolute.svg'); } h4 { background: url('../assets/component-img-relative.png'); } `, - // Use a small SVG for the absolute image to help validate that it is being referenced, - // because it is so small it would be inlined usually. 'src/assets/global-img-absolute.svg': imgSvg, 'src/assets/component-img-absolute.svg': imgSvg, }); @@ -427,12 +369,12 @@ describe('Browser Builder styles', () => { expect(styles).toContain(`url('global-img-relative.png')`); expect(main).toContain(`url('/assets/component-img-absolute.svg')`); expect(main).toContain(`url('component-img-relative.png')`); - expect(host.scopedSync().exists(normalize('dist/global-img-absolute.svg'))) - .toBe(false); + expect(host.scopedSync().exists(normalize('dist/assets/global-img-absolute.svg'))) + .toBe(true); expect(host.scopedSync().exists(normalize('dist/global-img-relative.png'))) .toBe(true); - expect(host.scopedSync().exists(normalize('dist/component-img-absolute.svg'))) - .toBe(false); + expect(host.scopedSync().exists(normalize('dist/assets/component-img-absolute.svg'))) + .toBe(true); expect(host.scopedSync().exists(normalize('dist/component-img-relative.png'))) .toBe(true); }), diff --git a/tests/legacy-cli/e2e/tests/build/styles/inline-urls.ts b/tests/legacy-cli/e2e/tests/build/styles/inline-urls.ts deleted file mode 100644 index a1ff323a10..0000000000 --- a/tests/legacy-cli/e2e/tests/build/styles/inline-urls.ts +++ /dev/null @@ -1,67 +0,0 @@ -import { ng, silentNpm } from '../../../utils/process'; -import { - expectFileToMatch, - expectFileToExist, - expectFileMatchToExist, - writeMultipleFiles -} from '../../../utils/fs'; -import { copyProjectAsset } from '../../../utils/assets'; -import { expectToFail } from '../../../utils/utils'; -import { updateJsonFile } from '../../../utils/project'; - -const imgSvg = ` - - - -`; - -export default function () { - // TODO(architect): Delete this test. It is now in devkit/build-angular. - - return Promise.resolve() - .then(() => silentNpm('install', 'font-awesome@4.7.0')) - .then(() => writeMultipleFiles({ - 'src/styles.scss': ` - $fa-font-path: "~font-awesome/fonts"; - @import "~font-awesome/scss/font-awesome"; - h1 { background: url('./assets/large.png'), - linear-gradient(to bottom, #0e40fa 25%, #0654f4 75%); } - h2 { background: url('./assets/small.svg'); } - p { background: url(./assets/small-id.svg#testID); } - `, - 'src/app/app.component.css': ` - h3 { background: url('../assets/small.svg'); } - h4 { background: url("../assets/large.png"); } - `, - 'src/assets/small.svg': imgSvg, - 'src/assets/small-id.svg': imgSvg - })) - .then(() => copyProjectAsset('images/spectrum.png', './src/assets/large.png')) - .then(() => updateJsonFile('angular.json', workspaceJson => { - const appArchitect = workspaceJson.projects['test-project'].targets; - appArchitect.build.options.styles = [ - { input: 'src/styles.scss' } - ]; - })) - .then(() => ng('build', '--extract-css', '--aot')) - .then(({ stdout }) => { - if (stdout.match(/postcss-url: \.+: Can't read file '\.+', ignoring/)) { - throw new Error('Expected no postcss-url file read warnings.'); - } - }) - // Check paths are correctly generated. - .then(() => expectFileToMatch('dist/test-project/styles.css', - /url\(['"]?large\.png['"]?\),\s+linear-gradient\(to bottom, #0e40fa 25%, #0654f4 75%\);/)) - .then(() => expectFileToMatch('dist/test-project/styles.css', - /url\(\\?['"]data:image\/svg\+xml/)) - .then(() => expectFileToMatch('dist/test-project/styles.css', - /url\(['"]?small-id\.svg#testID['"]?\)/)) - .then(() => expectFileToMatch('dist/test-project/main.js', - /url\(\\?['"]data:image\/svg\+xml/)) - .then(() => expectFileToMatch('dist/test-project/main.js', - /url\((?:['"]|\\')?large\.png(?:['"]|\\')?\)/)) - // Check files are correctly created. - .then(() => expectToFail(() => expectFileToExist('dist/test-project/small.svg'))) - .then(() => expectFileMatchToExist('./dist/test-project', /large\.png/)) - .then(() => expectFileMatchToExist('./dist/test-project', /small-id\.svg/)); -}