From a2e819a8dd7ee51f69c80550b3b71b053a13f503 Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Mon, 20 Feb 2017 13:18:21 +0000 Subject: [PATCH] fix(@angular/cli): fix css url processing (#4803) Fixing component css in #4667 uncovered errors in CSS url processing. This PR correctly composes absolute urls when using `--base-href` and/or `--deploy-url`. It also fixes asset output on `--aot` mode. Fix #4778 Fix #4782 Fix #4806 --- package.json | 1 + .../cli/models/webpack-configs/styles.ts | 40 ++++++++++--- packages/@angular/cli/package.json | 1 + packages/@angular/cli/tasks/eject.ts | 20 ++++++- .../@ngtools/webpack/src/resource_loader.ts | 8 ++- scripts/test-licenses.js | 5 +- tests/e2e/tests/build/css-urls.ts | 56 +++++++++++++++++++ 7 files changed, 117 insertions(+), 14 deletions(-) create mode 100644 tests/e2e/tests/build/css-urls.ts diff --git a/package.json b/package.json index 0da972783909..ca99417f576e 100644 --- a/package.json +++ b/package.json @@ -81,6 +81,7 @@ "opn": "4.0.2", "portfinder": "~1.0.12", "postcss-loader": "^0.13.0", + "postcss-url": "^5.1.2", "raw-loader": "^0.5.1", "resolve": "^1.1.7", "rimraf": "^2.5.3", diff --git a/packages/@angular/cli/models/webpack-configs/styles.ts b/packages/@angular/cli/models/webpack-configs/styles.ts index 1c40265bff84..532b0c8c99ed 100644 --- a/packages/@angular/cli/models/webpack-configs/styles.ts +++ b/packages/@angular/cli/models/webpack-configs/styles.ts @@ -1,13 +1,15 @@ import * as webpack from 'webpack'; import * as path from 'path'; +import { oneLineTrim } from 'common-tags'; import { SuppressExtractedTextChunksWebpackPlugin } from '../../plugins/suppress-entry-chunks-webpack-plugin'; import { extraEntryParser, getOutputHashFormat } from './utils'; import { WebpackConfigOptions } from '../webpack-config'; -import { pluginArgs } from '../../tasks/eject'; +import { pluginArgs, postcssArgs } from '../../tasks/eject'; const cssnano = require('cssnano'); +const postcssUrl = require('postcss-url'); const autoprefixer = require('autoprefixer'); const ExtractTextPlugin = require('extract-text-webpack-plugin'); @@ -39,11 +41,35 @@ export function getStylesConfig(wco: WebpackConfigOptions) { // https://github.com/webpack-contrib/style-loader#recommended-configuration const cssSourceMap = buildOptions.extractCss && buildOptions.sourcemap; - // minify/optimize css in production - // autoprefixer is always run separately so disable here - const extraPostCssPlugins = buildOptions.target === 'production' - ? [cssnano({ safe: true, autoprefixer: false })] - : []; + // Minify/optimize css in production. + const cssnanoPlugin = cssnano({ safe: true, autoprefixer: false }); + + // Convert absolute resource URLs to account for base-href and deploy-url. + const baseHref = wco.buildOptions.baseHref; + const deployUrl = wco.buildOptions.deployUrl; + const postcssUrlOptions = { + url: (URL: string) => { + // Only convert absolute URLs, which CSS-Loader won't process into require(). + if (!URL.startsWith('/')) { + return URL; + } + // Join together base-href, deploy-url and the original URL. + // Also dedupe multiple slashes into single ones. + return `/${baseHref || ''}/${deployUrl || ''}/${URL}`.replace(/\/\/+/g, '/'); + } + }; + const urlPlugin = postcssUrl(postcssUrlOptions); + // We need to save baseHref and deployUrl for the Ejected webpack config to work (we reuse + // the function defined above). + (postcssUrlOptions as any).baseHref = baseHref; + (postcssUrlOptions as any).deployUrl = deployUrl; + // Save the original options as arguments for eject. + urlPlugin[postcssArgs] = postcssUrlOptions; + + // PostCSS plugins. + const postCssPlugins = [autoprefixer(), urlPlugin].concat( + buildOptions.target === 'production' ? [cssnanoPlugin] : [] + ); // determine hashing format const hashFormat = getOutputHashFormat(buildOptions.outputHashing); @@ -141,7 +167,7 @@ export function getStylesConfig(wco: WebpackConfigOptions) { new webpack.LoaderOptionsPlugin({ sourceMap: cssSourceMap, options: { - postcss: [autoprefixer()].concat(extraPostCssPlugins), + postcss: postCssPlugins, // css-loader, stylus-loader don't support LoaderOptionsPlugin properly // options are in query instead sassLoader: { sourceMap: cssSourceMap, includePaths }, diff --git a/packages/@angular/cli/package.json b/packages/@angular/cli/package.json index 2df914d490a9..5b075bf7b18c 100644 --- a/packages/@angular/cli/package.json +++ b/packages/@angular/cli/package.json @@ -66,6 +66,7 @@ "opn": "4.0.2", "portfinder": "~1.0.12", "postcss-loader": "^0.13.0", + "postcss-url": "^5.1.2", "raw-loader": "^0.5.1", "resolve": "^1.1.7", "rimraf": "^2.5.3", diff --git a/packages/@angular/cli/tasks/eject.ts b/packages/@angular/cli/tasks/eject.ts index caa891004532..5a9a1bdfb8cb 100644 --- a/packages/@angular/cli/tasks/eject.ts +++ b/packages/@angular/cli/tasks/eject.ts @@ -18,6 +18,7 @@ const angularCliPlugins = require('../plugins/webpack'); const autoprefixer = require('autoprefixer'); +const postcssUrl = require('postcss-url'); const ExtractTextPlugin = require('extract-text-webpack-plugin'); const HtmlWebpackPlugin = require('html-webpack-plugin'); const SilentError = require('silent-error'); @@ -29,6 +30,7 @@ const ProgressPlugin = require('webpack/lib/ProgressPlugin'); export const pluginArgs = Symbol('plugin-args'); +export const postcssArgs = Symbol('postcss-args'); class JsonWebpackSerializer { @@ -135,6 +137,15 @@ class JsonWebpackSerializer { if (x && x.toString() == autoprefixer()) { this.variableImports['autoprefixer'] = 'autoprefixer'; return this._escape('autoprefixer()'); + } else if (x && x.toString() == postcssUrl()) { + this.variableImports['postcss-url'] = 'postcssUrl'; + let args = ''; + if (x[postcssArgs] && x[postcssArgs].url) { + this.variables['baseHref'] = JSON.stringify(x[postcssArgs].baseHref); + this.variables['deployUrl'] = JSON.stringify(x[postcssArgs].deployUrl); + args = `{"url": ${x[postcssArgs].url.toString()}}`; + } + return this._escape(`postcssUrl(${args})`); } else if (x && x.postcssPlugin == 'cssnano') { this.variableImports['cssnano'] = 'cssnano'; return this._escape('cssnano({ safe: true, autoprefixer: false })'); @@ -442,15 +453,18 @@ export default Task.extend({ packageJson['devDependencies']['webpack-dev-server'] = ourPackageJson['dependencies']['webpack-dev-server']; - // Update all loaders from webpack. + // Update all loaders from webpack, plus postcss plugins. [ + 'autoprefixer', 'css-loader', + 'cssnano', 'exports-loader', 'file-loader', 'json-loader', 'karma-sourcemap-loader', 'less-loader', 'postcss-loader', + 'postcss-url', 'raw-loader', 'sass-loader', 'script-loader', @@ -487,13 +501,13 @@ export default Task.extend({ console.log(yellow(stripIndent` ========================================================================================== Ejection was successful. - + To run your builds, you now need to do the following commands: - "npm run build" to build. - "npm run test" to run unit tests. - "npm start" to serve the app using webpack-dev-server. - "npm run e2e" to run protractor. - + Running the equivalent CLI commands will result in an error. ========================================================================================== diff --git a/packages/@ngtools/webpack/src/resource_loader.ts b/packages/@ngtools/webpack/src/resource_loader.ts index 9f9ff411e1a6..a1db3347476e 100644 --- a/packages/@ngtools/webpack/src/resource_loader.ts +++ b/packages/@ngtools/webpack/src/resource_loader.ts @@ -70,10 +70,12 @@ export class WebpackResourceLoader implements ResourceLoader { // Restore the parent compilation to the state like it was before the child compilation. Object.keys(childCompilation.assets).forEach((fileName) => { - this._parentCompilation.assets[fileName] = assetsBeforeCompilation[fileName]; - if (assetsBeforeCompilation[fileName] === undefined) { - // If it wasn't there - delete it. + // 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]; } }); diff --git a/scripts/test-licenses.js b/scripts/test-licenses.js index 7cef8219198d..83082cc493e6 100644 --- a/scripts/test-licenses.js +++ b/scripts/test-licenses.js @@ -64,6 +64,7 @@ const licenseReplacements = [ // TODO(hansl): review these const ignoredPackages = [ 'async-foreach@0.1.3', // MIT, but doesn't list it in package.json + 'directory-encoder@0.7.2', // MIT, but doesn't list it in package.json 'domelementtype@1.1.3', // Looks like MIT 'domelementtype@1.3.0', // Looks like MIT 'domhandler@2.1.0', // Looks like MIT @@ -79,8 +80,10 @@ const ignoredPackages = [ 'progress@1.1.8', // MIT, but doesn't list it in package.json 'samsam@1.1.2', // BSD, but doesn't list it in package.json 'stdout-stream@1.4.0', // MIT, but doesn't list it in package.json + 'uglify-js@2.3.6', // BSD, but doesn't list it in package.json 'undefined@undefined', // Test package with no name nor version. - 'verror@1.3.6' // Looks like MIT + 'verror@1.3.6', // Looks like MIT + 'xmldom@0.1.27' // LGPL,MIT but has a broken licenses array ]; const root = path.resolve(__dirname, '../'); diff --git a/tests/e2e/tests/build/css-urls.ts b/tests/e2e/tests/build/css-urls.ts new file mode 100644 index 000000000000..64559d8bd635 --- /dev/null +++ b/tests/e2e/tests/build/css-urls.ts @@ -0,0 +1,56 @@ +import { ng } from '../../utils/process'; +import { + expectFileToMatch, + expectFileToExist, + writeMultipleFiles +} from '../../utils/fs'; +import { expectToFail } from '../../utils/utils'; + +const imgSvg = ` + + + +`; + +export default function () { + return Promise.resolve() + // Verify absolute/relative paths in global/component css. + .then(() => writeMultipleFiles({ + 'src/styles.css': ` + h1 { background: url('/assets/global-img-absolute.svg'); } + h2 { background: url('./assets/global-img-relative.svg'); } + `, + 'src/app/app.component.css': ` + h3 { background: url('/assets/component-img-absolute.svg'); } + h4 { background: url('../assets/component-img-relative.svg'); } + `, + // Using SVGs because they are loaded via file-loader and thus never inlined. + 'src/assets/global-img-absolute.svg': imgSvg, + 'src/assets/global-img-relative.svg': imgSvg, + 'src/assets/component-img-absolute.svg': imgSvg, + 'src/assets/component-img-relative.svg': imgSvg + })) + .then(() => ng('build', '--extract-css', '--aot')) + // Check paths are correctly generated. + .then(() => expectFileToMatch('dist/styles.bundle.css', + `url\('\/assets\/global-img-absolute\.svg'\)`)) + .then(() => expectFileToMatch('dist/styles.bundle.css', 'url\(global-img-relative.svg\)')) + .then(() => expectFileToMatch('dist/main.bundle.js', + `url\(\\'\/assets\/component-img-absolute\.svg\\'\)`)) + .then(() => expectFileToMatch('dist/main.bundle.js', 'url\(component-img-relative\.svg\)')) + // Check files are correctly created. + .then(() => expectToFail(() => expectFileToExist('dist/global-img-absolute.svg'))) + .then(() => expectFileToExist('dist/global-img-relative.svg')) + .then(() => expectToFail(() => expectFileToExist('dist/component-img-absolute.svg'))) + .then(() => expectFileToExist('dist/component-img-relative.svg')) + // Also check with base-href and deploy-url flags. + .then(() => ng('build', '--base-href=/base/', '--deploy-url=deploy/', + '--extract-css', '--aot')) + .then(() => expectFileToMatch('dist/styles.bundle.css', + `url\('\/base\/deploy\/assets\/global-img-absolute\.svg'\)`)) + .then(() => expectFileToMatch('dist/styles.bundle.css', 'url\(global-img-relative.svg\)')) + .then(() => expectFileToMatch('dist/main.bundle.js', + `url\(\\'\/base\/deploy\/assets\/component-img-absolute\.svg\\'\)`)) + .then(() => expectFileToMatch('dist/main.bundle.js', + 'url\(deploy/component-img-relative\.svg\)')); +}