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

@@ -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",

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 },

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",

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.
 
           ==========================================================================================

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];
             }
           });
 

scripts/test-licenses.js

@@ -64,6 +64,7 @@ const licenseReplacements = [
 // TODO(hansl): review these
 const ignoredPackages = [
   '[email protected]', // MIT, but doesn't list it in package.json
+  '[email protected]', // MIT, but doesn't list it in package.json
   '[email protected]', // Looks like MIT
   '[email protected]', // Looks like MIT
   '[email protected]', // Looks like MIT
@@ -79,8 +80,10 @@ const ignoredPackages = [
   '[email protected]', // MIT, but doesn't list it in package.json
   '[email protected]', // BSD, but doesn't list it in package.json
   '[email protected]', // MIT, but doesn't list it in package.json
+  '[email protected]', // BSD, but doesn't list it in package.json
   'undefined@undefined', // Test package with no name nor version.
-  '[email protected]' // Looks like MIT
+  '[email protected]', // Looks like MIT
+  '[email protected]' // LGPL,MIT but has a broken licenses array
 ];
 
 const root = path.resolve(__dirname, '../');

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 = `
+  <svg width="100" height="100" xmlns="http://www.w3.org/2000/svg">
+    <circle cx="50" cy="50" r="40" stroke="green" stroke-width="4" fill="yellow" />
+  </svg>
+`;
+
+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\)'));
+}