From 04b3dee667649483797f6071b01b5363e9af2f27 Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Wed, 7 Oct 2015 13:37:56 -0700 Subject: [PATCH] fix(css): when compiling CSS, leave absolute imports alone Closes #4592 --- .../src/core/compiler/style_url_resolver.ts | 8 +++++++ .../angular2/src/core/dom/emulated_css.dart | 11 ++++++++++ .../test/core/compiler/shadow_css_spec.ts | 6 ++++++ .../core/compiler/style_url_resolver_spec.ts | 21 +++++++++++++++++++ .../src/transform/common/url_resolver.dart | 5 ++--- .../transform/common/url_resolver_tests.dart | 6 +++--- .../stylesheet_compiler/all_tests.dart | 17 +++++++++++++++ 7 files changed, 68 insertions(+), 6 deletions(-) diff --git a/modules/angular2/src/core/compiler/style_url_resolver.ts b/modules/angular2/src/core/compiler/style_url_resolver.ts index 71f1940492..b1acc89cbe 100644 --- a/modules/angular2/src/core/compiler/style_url_resolver.ts +++ b/modules/angular2/src/core/compiler/style_url_resolver.ts @@ -24,6 +24,11 @@ function extractUrls(resolver: UrlResolver, baseUrl: string, cssText: string, fo string { return StringWrapper.replaceAllMapped(cssText, _cssImportRe, (m) => { var url = isPresent(m[1]) ? m[1] : m[2]; + var schemeMatch = RegExpWrapper.firstMatch(_urlWithSchemaRe, url); + if (isPresent(schemeMatch) && schemeMatch[1] != 'package') { + // Do not attempt to resolve non-package absolute URLs with URI scheme + return m[0]; + } foundUrls.push(resolver.resolve(baseUrl, url)); return ''; }); @@ -50,3 +55,6 @@ var _cssUrlRe = /(url\()([^)]*)(\))/g; var _cssImportRe = /@import\s+(?:url\()?\s*(?:(?:['"]([^'"]*))|([^;\)\s]*))[^;]*;?/g; var _quoteRe = /['"]/g; var _dataUrlRe = /^['"]?data:/g; +// TODO: can't use /^[^:/?#.]+:/g due to clang-format bug: +// https://github.com/angular/angular/issues/4596 +var _urlWithSchemaRe = /^['"]?([a-zA-Z\-\+\.]+):/g; diff --git a/modules/angular2/src/core/dom/emulated_css.dart b/modules/angular2/src/core/dom/emulated_css.dart index e5ca210d3d..df62906afe 100644 --- a/modules/angular2/src/core/dom/emulated_css.dart +++ b/modules/angular2/src/core/dom/emulated_css.dart @@ -28,6 +28,8 @@ List emulateRules(Iterable rules) { return new EmulatedCssStyleRule(node); } else if (node is cssv.MediaDirective) { return new EmulatedCssMedialRule(node); + } else if (node is cssv.ImportDirective) { + return new EmulatedCssImportRule(node); } }) .where((r) => r != null) @@ -100,3 +102,12 @@ class EmulatedMediaList { .map((q) => q.span.text).join(' and '); } } + +/// Emulates [CSSImportRule](https://developer.mozilla.org/en-US/docs/Web/API/CSSImportRule) +class EmulatedCssImportRule extends EmulatedCssRule { + EmulatedCssImportRule(cssv.ImportDirective directive) { + this + ..type = 3 + ..cssText = '@${directive.span.text};'; + } +} diff --git a/modules/angular2/test/core/compiler/shadow_css_spec.ts b/modules/angular2/test/core/compiler/shadow_css_spec.ts index 00f84f25b9..1e9c334fd2 100644 --- a/modules/angular2/test/core/compiler/shadow_css_spec.ts +++ b/modules/angular2/test/core/compiler/shadow_css_spec.ts @@ -154,5 +154,11 @@ export function main() { var css = s('x >>> y {}', 'a'); expect(css).toEqual('x[a] y[a] {}'); }); + + it('should pass through @import directives', () => { + var styleStr = '@import url("https://fonts.googleapis.com/css?family=Roboto");'; + var css = s(styleStr, 'a'); + expect(css).toEqual(styleStr); + }); }); } diff --git a/modules/angular2/test/core/compiler/style_url_resolver_spec.ts b/modules/angular2/test/core/compiler/style_url_resolver_spec.ts index fb2f83813c..a906fd0964 100644 --- a/modules/angular2/test/core/compiler/style_url_resolver_spec.ts +++ b/modules/angular2/test/core/compiler/style_url_resolver_spec.ts @@ -88,5 +88,26 @@ export function main() { .toEqual(['http://ng.io/print1.css', 'http://ng.io/print2.css']); }); + it('should leave absolute non-package @import urls intact', () => { + var css = `@import url('http://server.com/some.css');`; + var styleWithImports = resolveStyleUrls(urlResolver, 'http://ng.io', css); + expect(styleWithImports.style.trim()).toEqual(`@import url('http://server.com/some.css');`); + expect(styleWithImports.styleUrls).toEqual([]); + }); + + it('should resolve package @import urls', () => { + var css = `@import url('package:a/b/some.css');`; + var styleWithImports = resolveStyleUrls(new FakeUrlResolver(), 'http://ng.io', css); + expect(styleWithImports.style.trim()).toEqual(``); + expect(styleWithImports.styleUrls).toEqual(['fake_resolved_url']); + }); + }); } + +/// The real thing behaves differently between Dart and JS for package URIs. +class FakeUrlResolver extends UrlResolver { + constructor() { super(); } + + resolve(baseUrl: string, url: string): string { return 'fake_resolved_url'; } +} diff --git a/modules_dart/transform/lib/src/transform/common/url_resolver.dart b/modules_dart/transform/lib/src/transform/common/url_resolver.dart index 18556de0a7..2f38188852 100644 --- a/modules_dart/transform/lib/src/transform/common/url_resolver.dart +++ b/modules_dart/transform/lib/src/transform/common/url_resolver.dart @@ -59,9 +59,8 @@ Uri toAssetScheme(Uri absoluteUri) { return absoluteUri; } if (absoluteUri.scheme != 'package') { - throw new FormatException( - 'Unsupported URI scheme "${absoluteUri.scheme}" encountered.', - absoluteUri); + // Pass through URIs with non-package scheme + return absoluteUri; } if (absoluteUri.pathSegments.length < 2) { diff --git a/modules_dart/transform/test/transform/common/url_resolver_tests.dart b/modules_dart/transform/test/transform/common/url_resolver_tests.dart index 4deffa16b5..47186d70c0 100644 --- a/modules_dart/transform/test/transform/common/url_resolver_tests.dart +++ b/modules_dart/transform/test/transform/common/url_resolver_tests.dart @@ -88,9 +88,9 @@ void allTests() { .toThrowWith(anInstanceOf: FormatException); }); - it('should throw for unsupported schemes', () { - expect(() => toAssetScheme(Uri.parse('file:///angular2'))) - .toThrowWith(anInstanceOf: FormatException); + it('should pass through unsupported schemes', () { + var uri = 'http://server.com/style.css'; + expect('${toAssetScheme(Uri.parse(uri))}').toEqual(uri); }); it('should throw if passed a null uri', () { diff --git a/modules_dart/transform/test/transform/stylesheet_compiler/all_tests.dart b/modules_dart/transform/test/transform/stylesheet_compiler/all_tests.dart index 39328e4866..6ed9ec218d 100644 --- a/modules_dart/transform/test/transform/stylesheet_compiler/all_tests.dart +++ b/modules_dart/transform/test/transform/stylesheet_compiler/all_tests.dart @@ -16,6 +16,9 @@ const SIMPLE_CSS = ''' } '''; +const HTTP_IMPORT = 'https://fonts.googleapis.com/css?family=Roboto'; +const CSS_WITH_IMPORT = '@import url(${HTTP_IMPORT});'; + main() { Html5LibDomAdapter.makeCurrent(); allTests(); @@ -60,6 +63,20 @@ allTests() { expect(transform.outputs[1].id.toString()) .toEqual('somepackage|lib/style.css.shim.dart'); }); + + it('should compile stylesheets with imports', () async { + var cssFile = new Asset.fromString( + new AssetId('somepackage', 'lib/style.css'), CSS_WITH_IMPORT); + var transform = new FakeTransform()..primaryInput = cssFile; + await subject.apply(transform); + expect(transform.outputs.length).toBe(2); + expect(transform.outputs[0].id.toString()) + .toEqual('somepackage|lib/style.css.dart'); + expect(transform.outputs[1].id.toString()) + .toEqual('somepackage|lib/style.css.shim.dart'); + expect(await transform.outputs[0].readAsString()).toContain(HTTP_IMPORT); + expect(await transform.outputs[1].readAsString()).toContain(HTTP_IMPORT); + }); } @proxy