Skip to content

Commit ba6bc57

Browse files
filipesilvaZhicheng Wang
filipesilva
authored and
Zhicheng Wang
committed
fix(@angular/cli): fix css url processing (angular#4803)
Fixing component css in angular#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 angular#4778 Fix angular#4782 Fix angular#4806
1 parent 96bb4f2 commit ba6bc57

File tree

7 files changed

+117
-14
lines changed

7 files changed

+117
-14
lines changed

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@
8181
"opn": "4.0.2",
8282
"portfinder": "~1.0.12",
8383
"postcss-loader": "^0.13.0",
84+
"postcss-url": "^5.1.2",
8485
"raw-loader": "^0.5.1",
8586
"resolve": "^1.1.7",
8687
"rimraf": "^2.5.3",

‎packages/@angular/cli/models/webpack-configs/styles.ts‎

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
import * as webpack from 'webpack';
22
import * as path from 'path';
3+
import { oneLineTrim } from 'common-tags';
34
import {
45
SuppressExtractedTextChunksWebpackPlugin
56
} from '../../plugins/suppress-entry-chunks-webpack-plugin';
67
import { extraEntryParser, getOutputHashFormat } from './utils';
78
import { WebpackConfigOptions } from '../webpack-config';
8-
import { pluginArgs } from '../../tasks/eject';
9+
import { pluginArgs, postcssArgs } from '../../tasks/eject';
910

1011
const cssnano = require('cssnano');
12+
const postcssUrl = require('postcss-url');
1113
const autoprefixer = require('autoprefixer');
1214
const ExtractTextPlugin = require('extract-text-webpack-plugin');
1315

@@ -39,11 +41,35 @@ export function getStylesConfig(wco: WebpackConfigOptions) {
3941
// https://github.com/webpack-contrib/style-loader#recommended-configuration
4042
const cssSourceMap = buildOptions.sourcemap;
4143

42-
// minify/optimize css in production
43-
// autoprefixer is always run separately so disable here
44-
const extraPostCssPlugins = buildOptions.target === 'production'
45-
? [cssnano({ safe: true, autoprefixer: false })]
46-
: [];
44+
// Minify/optimize css in production.
45+
const cssnanoPlugin = cssnano({ safe: true, autoprefixer: false });
46+
47+
// Convert absolute resource URLs to account for base-href and deploy-url.
48+
const baseHref = wco.buildOptions.baseHref;
49+
const deployUrl = wco.buildOptions.deployUrl;
50+
const postcssUrlOptions = {
51+
url: (URL: string) => {
52+
// Only convert absolute URLs, which CSS-Loader won't process into require().
53+
if (!URL.startsWith('/')) {
54+
return URL;
55+
}
56+
// Join together base-href, deploy-url and the original URL.
57+
// Also dedupe multiple slashes into single ones.
58+
return `/${baseHref || ''}/${deployUrl || ''}/${URL}`.replace(/\/\/+/g, '/');
59+
}
60+
};
61+
const urlPlugin = postcssUrl(postcssUrlOptions);
62+
// We need to save baseHref and deployUrl for the Ejected webpack config to work (we reuse
63+
// the function defined above).
64+
(postcssUrlOptions as any).baseHref = baseHref;
65+
(postcssUrlOptions as any).deployUrl = deployUrl;
66+
// Save the original options as arguments for eject.
67+
urlPlugin[postcssArgs] = postcssUrlOptions;
68+
69+
// PostCSS plugins.
70+
const postCssPlugins = [autoprefixer(), urlPlugin].concat(
71+
buildOptions.target === 'production' ? [cssnanoPlugin] : []
72+
);
4773

4874
// determine hashing format
4975
const hashFormat = getOutputHashFormat(buildOptions.outputHashing);
@@ -142,7 +168,7 @@ export function getStylesConfig(wco: WebpackConfigOptions) {
142168
new webpack.LoaderOptionsPlugin({
143169
sourceMap: cssSourceMap,
144170
options: {
145-
postcss: [autoprefixer()].concat(extraPostCssPlugins),
171+
postcss: postCssPlugins,
146172
// css-loader, stylus-loader don't support LoaderOptionsPlugin properly
147173
// options are in query instead
148174
sassLoader: { sourceMap: cssSourceMap, includePaths },

‎packages/@angular/cli/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@
6969
"opn": "4.0.2",
7070
"portfinder": "~1.0.12",
7171
"postcss-loader": "^0.13.0",
72+
"postcss-url": "^5.1.2",
7273
"raw-loader": "^0.5.1",
7374
"resolve": "^1.1.7",
7475
"rimraf": "^2.5.3",

‎packages/@angular/cli/tasks/eject.ts‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ const angularCliPlugins = require('../plugins/webpack');
1818

1919

2020
const autoprefixer = require('autoprefixer');
21+
const postcssUrl = require('postcss-url');
2122
const ExtractTextPlugin = require('extract-text-webpack-plugin');
2223
const HtmlWebpackPlugin = require('html-webpack-plugin');
2324
const SilentError = require('silent-error');
@@ -29,6 +30,7 @@ const ProgressPlugin = require('webpack/lib/ProgressPlugin');
2930

3031

3132
export const pluginArgs = Symbol('plugin-args');
33+
export const postcssArgs = Symbol('postcss-args');
3234

3335

3436
class JsonWebpackSerializer {
@@ -135,6 +137,15 @@ class JsonWebpackSerializer {
135137
if (x && x.toString() == autoprefixer()) {
136138
this.variableImports['autoprefixer'] = 'autoprefixer';
137139
return this._escape('autoprefixer()');
140+
} else if (x && x.toString() == postcssUrl()) {
141+
this.variableImports['postcss-url'] = 'postcssUrl';
142+
let args = '';
143+
if (x[postcssArgs] && x[postcssArgs].url) {
144+
this.variables['baseHref'] = JSON.stringify(x[postcssArgs].baseHref);
145+
this.variables['deployUrl'] = JSON.stringify(x[postcssArgs].deployUrl);
146+
args = `{"url": ${x[postcssArgs].url.toString()}}`;
147+
}
148+
return this._escape(`postcssUrl(${args})`);
138149
} else if (x && x.postcssPlugin == 'cssnano') {
139150
this.variableImports['cssnano'] = 'cssnano';
140151
return this._escape('cssnano({ safe: true, autoprefixer: false })');
@@ -442,15 +453,18 @@ export default Task.extend({
442453
packageJson['devDependencies']['webpack-dev-server']
443454
= ourPackageJson['dependencies']['webpack-dev-server'];
444455

445-
// Update all loaders from webpack.
456+
// Update all loaders from webpack, plus postcss plugins.
446457
[
458+
'autoprefixer',
447459
'css-loader',
460+
'cssnano',
448461
'exports-loader',
449462
'file-loader',
450463
'json-loader',
451464
'karma-sourcemap-loader',
452465
'less-loader',
453466
'postcss-loader',
467+
'postcss-url',
454468
'raw-loader',
455469
'sass-loader',
456470
'script-loader',
@@ -487,13 +501,13 @@ export default Task.extend({
487501
console.log(yellow(stripIndent`
488502
==========================================================================================
489503
Ejection was successful.
490-
504+
491505
To run your builds, you now need to do the following commands:
492506
- "npm run build" to build.
493507
- "npm run test" to run unit tests.
494508
- "npm start" to serve the app using webpack-dev-server.
495509
- "npm run e2e" to run protractor.
496-
510+
497511
Running the equivalent CLI commands will result in an error.
498512
499513
==========================================================================================

‎packages/@ngtools/webpack/src/resource_loader.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,12 @@ export class WebpackResourceLoader implements ResourceLoader {
7070

7171
// Restore the parent compilation to the state like it was before the child compilation.
7272
Object.keys(childCompilation.assets).forEach((fileName) => {
73-
this._parentCompilation.assets[fileName] = assetsBeforeCompilation[fileName];
74-
if (assetsBeforeCompilation[fileName] === undefined) {
75-
// If it wasn't there - delete it.
73+
// If it wasn't there and it's a source file (absolute path) - delete it.
74+
if (assetsBeforeCompilation[fileName] === undefined && path.isAbsolute(fileName)) {
7675
delete this._parentCompilation.assets[fileName];
76+
} else {
77+
// Otherwise, add it to the parent compilation.
78+
this._parentCompilation.assets[fileName] = childCompilation.assets[fileName];
7779
}
7880
});
7981

‎scripts/test-licenses.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ const licenseReplacements = [
6464
// TODO(hansl): review these
6565
const ignoredPackages = [
6666
'async-foreach@0.1.3', // MIT, but doesn't list it in package.json
67+
'directory-encoder@0.7.2', // MIT, but doesn't list it in package.json
6768
'domelementtype@1.1.3', // Looks like MIT
6869
'domelementtype@1.3.0', // Looks like MIT
6970
'domhandler@2.1.0', // Looks like MIT
@@ -79,8 +80,10 @@ const ignoredPackages = [
7980
'progress@1.1.8', // MIT, but doesn't list it in package.json
8081
'samsam@1.1.2', // BSD, but doesn't list it in package.json
8182
'stdout-stream@1.4.0', // MIT, but doesn't list it in package.json
83+
'uglify-js@2.3.6', // BSD, but doesn't list it in package.json
8284
'undefined@undefined', // Test package with no name nor version.
83-
'verror@1.3.6' // Looks like MIT
85+
'verror@1.3.6', // Looks like MIT
86+
'xmldom@0.1.27' // LGPL,MIT but has a broken licenses array
8487
];
8588

8689
const root = path.resolve(__dirname, '../');

‎tests/e2e/tests/build/css-urls.ts‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import { ng } from '../../utils/process';
2+
import {
3+
expectFileToMatch,
4+
expectFileToExist,
5+
writeMultipleFiles
6+
} from '../../utils/fs';
7+
import { expectToFail } from '../../utils/utils';
8+
9+
const imgSvg = `
10+
<svg width="100" height="100" xmlns="http://www.w3.org/2000/svg">
11+
<circle cx="50" cy="50" r="40" stroke="green" stroke-width="4" fill="yellow" />
12+
</svg>
13+
`;
14+
15+
export default function () {
16+
return Promise.resolve()
17+
// Verify absolute/relative paths in global/component css.
18+
.then(() => writeMultipleFiles({
19+
'src/styles.css': `
20+
h1 { background: url('/assets/global-img-absolute.svg'); }
21+
h2 { background: url('./assets/global-img-relative.svg'); }
22+
`,
23+
'src/app/app.component.css': `
24+
h3 { background: url('/assets/component-img-absolute.svg'); }
25+
h4 { background: url('../assets/component-img-relative.svg'); }
26+
`,
27+
// Using SVGs because they are loaded via file-loader and thus never inlined.
28+
'src/assets/global-img-absolute.svg': imgSvg,
29+
'src/assets/global-img-relative.svg': imgSvg,
30+
'src/assets/component-img-absolute.svg': imgSvg,
31+
'src/assets/component-img-relative.svg': imgSvg
32+
}))
33+
.then(() => ng('build', '--extract-css', '--aot'))
34+
// Check paths are correctly generated.
35+
.then(() => expectFileToMatch('dist/styles.bundle.css',
36+
`url\('\/assets\/global-img-absolute\.svg'\)`))
37+
.then(() => expectFileToMatch('dist/styles.bundle.css', 'url\(global-img-relative.svg\)'))
38+
.then(() => expectFileToMatch('dist/main.bundle.js',
39+
`url\(\\'\/assets\/component-img-absolute\.svg\\'\)`))
40+
.then(() => expectFileToMatch('dist/main.bundle.js', 'url\(component-img-relative\.svg\)'))
41+
// Check files are correctly created.
42+
.then(() => expectToFail(() => expectFileToExist('dist/global-img-absolute.svg')))
43+
.then(() => expectFileToExist('dist/global-img-relative.svg'))
44+
.then(() => expectToFail(() => expectFileToExist('dist/component-img-absolute.svg')))
45+
.then(() => expectFileToExist('dist/component-img-relative.svg'))
46+
// Also check with base-href and deploy-url flags.
47+
.then(() => ng('build', '--base-href=/base/', '--deploy-url=deploy/',
48+
'--extract-css', '--aot'))
49+
.then(() => expectFileToMatch('dist/styles.bundle.css',
50+
`url\('\/base\/deploy\/assets\/global-img-absolute\.svg'\)`))
51+
.then(() => expectFileToMatch('dist/styles.bundle.css', 'url\(global-img-relative.svg\)'))
52+
.then(() => expectFileToMatch('dist/main.bundle.js',
53+
`url\(\\'\/base\/deploy\/assets\/component-img-absolute\.svg\\'\)`))
54+
.then(() => expectFileToMatch('dist/main.bundle.js',
55+
'url\(deploy/component-img-relative\.svg\)'));
56+
}

0 commit comments

Comments
 (0)