Skip to content

Commit

Permalink
fix(@angular/cli): correctly compose absolute urls
Browse files Browse the repository at this point in the history
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`.

Fix #4778
Fix #4782
  • Loading branch information
filipesilva committed Feb 18, 2017
1 parent d2bef98 commit 3fadf6a
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 8 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
33 changes: 27 additions & 6 deletions packages/@angular/cli/models/webpack-configs/styles.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as webpack from 'webpack';
import * as path from 'path';
import { oneLineTrim } from 'common-tags';
import {
SuppressExtractedTextChunksWebpackPlugin
} from '../../plugins/suppress-entry-chunks-webpack-plugin';
Expand All @@ -8,6 +9,7 @@ import { WebpackConfigOptions } from '../webpack-config';
import { pluginArgs } from '../../tasks/eject';

const cssnano = require('cssnano');
const postcssUrl = require('postcss-url');
const autoprefixer = require('autoprefixer');
const ExtractTextPlugin = require('extract-text-webpack-plugin');

Expand Down Expand Up @@ -39,11 +41,30 @@ 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 urlPlugin = postcssUrl({
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 oneLineTrim`
/${wco.buildOptions.baseHref || ''}
/${wco.buildOptions.deployUrl || ''}
/${URL}
`.replace(/\/\/+/g, '/');
}
});

// PostCSS plugins.
const postCssPlugins = [autoprefixer(), urlPlugin].concat(
buildOptions.target === 'production' ? [cssnanoPlugin] : []
);

// determine hashing format
const hashFormat = getOutputHashFormat(buildOptions.outputHashing);
Expand Down Expand Up @@ -141,7 +162,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 },
Expand Down
1 change: 1 addition & 0 deletions packages/@angular/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
8 changes: 6 additions & 2 deletions packages/@angular/cli/tasks/eject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -135,6 +136,9 @@ 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';
return this._escape('postcssUrl()');
} else if (x && x.postcssPlugin == 'cssnano') {
this.variableImports['cssnano'] = 'cssnano';
return this._escape('cssnano({ safe: true, autoprefixer: false })');
Expand Down Expand Up @@ -487,13 +491,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.
==========================================================================================
Expand Down
43 changes: 43 additions & 0 deletions tests/e2e/tests/build/css-urls.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { ng } from '../../utils/process';
import { expectFileToMatch, writeMultipleFiles } from '../../utils/fs';
import { stripIndents } from 'common-tags';


export default function () {
return Promise.resolve()
// Verify absolute/relative paths in global/component css.
.then(() => writeMultipleFiles({
'src/styles.css': `
h1 { background: url('/assets/img-absolute.svg'); }
h2 { background: url('./assets/img-relative.svg'); }
`,
'src/app/app.component.css': `
h3 { background: url('/assets/img-absolute.svg'); }
h4 { background: url('../assets/img-relative.svg'); }
`,
// Using SVGs because they are loaded via file-loader and thus never inlined.
'src/assets/img-absolute.svg': stripIndents`
<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>
`,
'src/assets/img-relative.svg': stripIndents`
<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>
`}))
.then(() => ng('build', '--extract-css'))
.then(() => expectFileToMatch('dist/styles.bundle.css', `url\('\/assets\/img-absolute\.svg'\)`))
.then(() => expectFileToMatch('dist/styles.bundle.css', 'url\(img-relative.svg\)'))
.then(() => expectFileToMatch('dist/main.bundle.js', `url\('\/assets\/img-absolute\.svg'\)`))
.then(() => expectFileToMatch('dist/main.bundle.js', 'background: url\(" \+ __webpack_require'))
// Also check with base-href and deploy-url flags.
.then(() => ng('build', '--base-href=/base/', '--deploy-url=/deploy/', '--extract-css'))
.then(() => expectFileToMatch('dist/styles.bundle.css',
`url\('\/base\/deploy\/assets\/img-absolute\.svg'\)`))
.then(() => expectFileToMatch('dist/styles.bundle.css', 'url\(img-relative.svg\)'))
.then(() => expectFileToMatch('dist/main.bundle.js',
`url\('\/base\/deploy\/assets\/img-absolute\.svg'\)`))
.then(() => expectFileToMatch('dist/main.bundle.js',
'background: url\(" \+ __webpack_require'));
}

0 comments on commit 3fadf6a

Please sign in to comment.