Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(@angular/cli): fix css url processing #4803

Merged
merged 2 commits into from
Feb 20, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
40 changes: 33 additions & 7 deletions packages/@angular/cli/models/webpack-configs/styles.ts
Original file line number Diff line number Diff line change
@@ -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');

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 },
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
20 changes: 17 additions & 3 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 All @@ -29,6 +30,7 @@ const ProgressPlugin = require('webpack/lib/ProgressPlugin');


export const pluginArgs = Symbol('plugin-args');
export const postcssArgs = Symbol('postcss-args');


class JsonWebpackSerializer {
Expand Down Expand Up @@ -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 })');
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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.

==========================================================================================
Expand Down
8 changes: 5 additions & 3 deletions packages/@ngtools/webpack/src/resource_loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
});

Expand Down
5 changes: 4 additions & 1 deletion scripts/test-licenses.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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, '../');
Expand Down
56 changes: 56 additions & 0 deletions tests/e2e/tests/build/css-urls.ts
Original file line number Diff line number Diff line change
@@ -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\)'));
}