From 1c4d233fe7f940256dc353ec4a71a2f7be492714 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Thu, 18 Jun 2015 10:07:43 +0200 Subject: [PATCH] fix(ShadowDomStrategy): always inline import rules fixes #1694 --- modules/angular2/src/core/application.ts | 6 +-- .../emulated_scoped_shadow_dom_strategy.ts | 24 +++++---- .../emulated_unscoped_shadow_dom_strategy.ts | 21 ++++++-- .../shadow_dom/native_shadow_dom_strategy.ts | 17 +++++-- .../angular2/src/test_lib/test_injector.ts | 6 +-- ...lated_unscoped_shadow_dom_strategy_spec.ts | 51 ++++++++++++++++--- .../native_shadow_dom_strategy_spec.ts | 51 ++++++++++++++++--- .../shadow_dom_emulation_integration_spec.ts | 12 ++--- .../src/compiler/compiler_benchmark.ts | 4 +- 9 files changed, 149 insertions(+), 43 deletions(-) diff --git a/modules/angular2/src/core/application.ts b/modules/angular2/src/core/application.ts index 7fbbc9061f..9781e5b5b9 100644 --- a/modules/angular2/src/core/application.ts +++ b/modules/angular2/src/core/application.ts @@ -108,9 +108,9 @@ function _injectorBindings(appComponentType): List> { }, [NgZone]), bind(ShadowDomStrategy) - .toFactory((styleUrlResolver, doc) => - new EmulatedUnscopedShadowDomStrategy(styleUrlResolver, doc.head), - [StyleUrlResolver, DOCUMENT_TOKEN]), + .toFactory((styleInliner, styleUrlResolver, doc) => new EmulatedUnscopedShadowDomStrategy( + styleInliner, styleUrlResolver, doc.head), + [StyleInliner, StyleUrlResolver, DOCUMENT_TOKEN]), DomRenderer, DefaultDomCompiler, bind(Renderer).toAlias(DomRenderer), diff --git a/modules/angular2/src/render/dom/shadow_dom/emulated_scoped_shadow_dom_strategy.ts b/modules/angular2/src/render/dom/shadow_dom/emulated_scoped_shadow_dom_strategy.ts index c124b68630..5d11509ef2 100644 --- a/modules/angular2/src/render/dom/shadow_dom/emulated_scoped_shadow_dom_strategy.ts +++ b/modules/angular2/src/render/dom/shadow_dom/emulated_scoped_shadow_dom_strategy.ts @@ -27,8 +27,11 @@ import { * - see `ShadowCss` for more information and limitations. */ export class EmulatedScopedShadowDomStrategy extends EmulatedUnscopedShadowDomStrategy { - constructor(public styleInliner: StyleInliner, styleUrlResolver: StyleUrlResolver, styleHost) { - super(styleUrlResolver, styleHost); + styleInliner: StyleInliner; + + constructor(styleInliner: StyleInliner, styleUrlResolver: StyleUrlResolver, styleHost) { + super(styleInliner, styleUrlResolver, styleHost); + this.styleInliner = styleInliner; } processStyleElement(hostComponentId: string, templateUrl: string, styleEl): Promise { @@ -37,20 +40,21 @@ export class EmulatedScopedShadowDomStrategy extends EmulatedUnscopedShadowDomSt cssText = this.styleUrlResolver.resolveUrls(cssText, templateUrl); var inlinedCss = this.styleInliner.inlineImports(cssText, templateUrl); + var ret = null; if (isPromise(inlinedCss)) { DOM.setText(styleEl, ''); - return (>inlinedCss) - .then((css) => { - css = shimCssForComponent(css, hostComponentId); - DOM.setText(styleEl, css); - this._moveToStyleHost(styleEl); - }); + ret = (>inlinedCss) + .then((css) => { + css = shimCssForComponent(css, hostComponentId); + DOM.setText(styleEl, css); + }); } else { var css = shimCssForComponent(inlinedCss, hostComponentId); DOM.setText(styleEl, css); - this._moveToStyleHost(styleEl); - return null; } + + this._moveToStyleHost(styleEl); + return ret; } _moveToStyleHost(styleEl) { diff --git a/modules/angular2/src/render/dom/shadow_dom/emulated_unscoped_shadow_dom_strategy.ts b/modules/angular2/src/render/dom/shadow_dom/emulated_unscoped_shadow_dom_strategy.ts index f747836468..e724726b18 100644 --- a/modules/angular2/src/render/dom/shadow_dom/emulated_unscoped_shadow_dom_strategy.ts +++ b/modules/angular2/src/render/dom/shadow_dom/emulated_unscoped_shadow_dom_strategy.ts @@ -1,3 +1,4 @@ +import {isPromise} from 'angular2/src/facade/lang'; import {Promise} from 'angular2/src/facade/async'; import {DOM} from 'angular2/src/dom/dom_adapter'; @@ -7,6 +8,7 @@ import * as viewModule from '../view/view'; import {LightDom} from './light_dom'; import {ShadowDomStrategy} from './shadow_dom_strategy'; import {StyleUrlResolver} from './style_url_resolver'; +import {StyleInliner} from 'angular2/src/render/dom/shadow_dom/style_inliner'; import {insertSharedStyleText} from './util'; /** @@ -19,7 +21,10 @@ import {insertSharedStyleText} from './util'; * - you can **not** use shadow DOM specific selectors in the styles */ export class EmulatedUnscopedShadowDomStrategy extends ShadowDomStrategy { - constructor(public styleUrlResolver: StyleUrlResolver, public styleHost) { super(); } + constructor(public styleInliner: StyleInliner, public styleUrlResolver: StyleUrlResolver, + public styleHost) { + super(); + } hasNativeContentElement(): boolean { return false; } @@ -31,11 +36,19 @@ export class EmulatedUnscopedShadowDomStrategy extends ShadowDomStrategy { processStyleElement(hostComponentId: string, templateUrl: string, styleEl): Promise { var cssText = DOM.getText(styleEl); + cssText = this.styleUrlResolver.resolveUrls(cssText, templateUrl); - DOM.setText(styleEl, cssText); - DOM.remove(styleEl); + var inlinedCss = this.styleInliner.inlineImports(cssText, templateUrl); + + var ret = null; + if (isPromise(inlinedCss)) { + DOM.setText(styleEl, ''); + ret = (>inlinedCss).then(css => { DOM.setText(styleEl, css); }); + } else { + DOM.setText(styleEl, inlinedCss); + } insertSharedStyleText(cssText, this.styleHost, styleEl); - return null; + return ret; } } diff --git a/modules/angular2/src/render/dom/shadow_dom/native_shadow_dom_strategy.ts b/modules/angular2/src/render/dom/shadow_dom/native_shadow_dom_strategy.ts index 4716afa659..0904cf4945 100644 --- a/modules/angular2/src/render/dom/shadow_dom/native_shadow_dom_strategy.ts +++ b/modules/angular2/src/render/dom/shadow_dom/native_shadow_dom_strategy.ts @@ -1,3 +1,4 @@ +import {isPromise} from 'angular2/src/facade/lang'; import {Promise} from 'angular2/src/facade/async'; import {Injectable} from 'angular2/di'; @@ -5,6 +6,7 @@ import {DOM} from 'angular2/src/dom/dom_adapter'; import {StyleUrlResolver} from './style_url_resolver'; import {ShadowDomStrategy} from './shadow_dom_strategy'; +import {StyleInliner} from 'angular2/src/render/dom/shadow_dom/style_inliner'; /** * This strategies uses the native Shadow DOM support. @@ -14,14 +16,23 @@ import {ShadowDomStrategy} from './shadow_dom_strategy'; */ @Injectable() export class NativeShadowDomStrategy extends ShadowDomStrategy { - constructor(public styleUrlResolver: StyleUrlResolver) { super(); } + constructor(public styleInliner: StyleInliner, public styleUrlResolver: StyleUrlResolver) { + super(); + } prepareShadowRoot(el) { return DOM.createShadowRoot(el); } processStyleElement(hostComponentId: string, templateUrl: string, styleEl): Promise { var cssText = DOM.getText(styleEl); + cssText = this.styleUrlResolver.resolveUrls(cssText, templateUrl); - DOM.setText(styleEl, cssText); - return null; + var inlinedCss = this.styleInliner.inlineImports(cssText, templateUrl); + + if (isPromise(inlinedCss)) { + return (>inlinedCss).then(css => { DOM.setText(styleEl, css); }); + } else { + DOM.setText(styleEl, inlinedCss); + return null; + } } } diff --git a/modules/angular2/src/test_lib/test_injector.ts b/modules/angular2/src/test_lib/test_injector.ts index 21b96e96c9..64f1a90fcc 100644 --- a/modules/angular2/src/test_lib/test_injector.ts +++ b/modules/angular2/src/test_lib/test_injector.ts @@ -85,9 +85,9 @@ function _getAppBindings() { bind(DOCUMENT_TOKEN) .toValue(appDoc), bind(ShadowDomStrategy) - .toFactory((styleUrlResolver, doc) => - new EmulatedUnscopedShadowDomStrategy(styleUrlResolver, doc.head), - [StyleUrlResolver, DOCUMENT_TOKEN]), + .toFactory((styleInliner, styleUrlResolver, doc) => new EmulatedUnscopedShadowDomStrategy( + styleInliner, styleUrlResolver, doc.head), + [StyleInliner, StyleUrlResolver, DOCUMENT_TOKEN]), DomRenderer, DefaultDomCompiler, bind(Renderer).toAlias(DomRenderer), diff --git a/modules/angular2/test/render/dom/shadow_dom/emulated_unscoped_shadow_dom_strategy_spec.ts b/modules/angular2/test/render/dom/shadow_dom/emulated_unscoped_shadow_dom_strategy_spec.ts index 01f4a52ca0..253fca4a30 100644 --- a/modules/angular2/test/render/dom/shadow_dom/emulated_unscoped_shadow_dom_strategy_spec.ts +++ b/modules/angular2/test/render/dom/shadow_dom/emulated_unscoped_shadow_dom_strategy_spec.ts @@ -23,18 +23,26 @@ import { } from 'angular2/src/render/dom/shadow_dom/util'; import {UrlResolver} from 'angular2/src/services/url_resolver'; import {StyleUrlResolver} from 'angular2/src/render/dom/shadow_dom/style_url_resolver'; +import {StyleInliner} from 'angular2/src/render/dom/shadow_dom/style_inliner'; + +import {isBlank} from 'angular2/src/facade/lang'; +import {PromiseWrapper, Promise} from 'angular2/src/facade/async'; + +import {XHR} from 'angular2/src/render/xhr'; export function main() { var strategy; describe('EmulatedUnscopedShadowDomStrategy', () => { - var styleHost; + var xhr, styleHost; beforeEach(() => { var urlResolver = new UrlResolver(); var styleUrlResolver = new StyleUrlResolver(urlResolver); + xhr = new FakeXHR(); + var styleInliner = new StyleInliner(xhr, styleUrlResolver, urlResolver); styleHost = el('
'); - strategy = new EmulatedUnscopedShadowDomStrategy(styleUrlResolver, styleHost); + strategy = new EmulatedUnscopedShadowDomStrategy(styleInliner, styleUrlResolver, styleHost); resetShadowDomCache(); }); @@ -49,11 +57,20 @@ export function main() { expect(styleElement).toHaveText(".foo {background-image: url('http://base/img.jpg');}"); }); - it('should not inline import rules', () => { - var styleElement = el(''); - strategy.processStyleElement('someComponent', 'http://base', styleElement); - expect(styleElement).toHaveText("@import 'http://base/other.css';"); - }); + it('should inline @import rules', inject([AsyncTestCompleter], (async) => { + xhr.reply('http://base/one.css', '.one {}'); + + var styleElement = el(''); + var stylePromise = + strategy.processStyleElement('someComponent', 'http://base', styleElement); + expect(stylePromise).toBePromise(); + expect(styleElement).toHaveText(''); + + stylePromise.then((_) => { + expect(styleElement).toHaveText('.one {}\n'); + async.done(); + }); + })); it('should move the style element to the style host', () => { var compileElement = el('
'); @@ -79,3 +96,23 @@ export function main() { }); } + +class FakeXHR extends XHR { + _responses: Map; + + constructor() { + super(); + this._responses = MapWrapper.create(); + } + + get(url: string): Promise { + var response = MapWrapper.get(this._responses, url); + if (isBlank(response)) { + return PromiseWrapper.reject('xhr error', null); + } + + return PromiseWrapper.resolve(response); + } + + reply(url: string, response: string) { MapWrapper.set(this._responses, url, response); } +} diff --git a/modules/angular2/test/render/dom/shadow_dom/native_shadow_dom_strategy_spec.ts b/modules/angular2/test/render/dom/shadow_dom/native_shadow_dom_strategy_spec.ts index eb656d4f4e..4b762185a3 100644 --- a/modules/angular2/test/render/dom/shadow_dom/native_shadow_dom_strategy_spec.ts +++ b/modules/angular2/test/render/dom/shadow_dom/native_shadow_dom_strategy_spec.ts @@ -17,6 +17,13 @@ import { } from 'angular2/src/render/dom/shadow_dom/native_shadow_dom_strategy'; import {UrlResolver} from 'angular2/src/services/url_resolver'; import {StyleUrlResolver} from 'angular2/src/render/dom/shadow_dom/style_url_resolver'; +import {StyleInliner} from 'angular2/src/render/dom/shadow_dom/style_inliner'; + +import {XHR} from 'angular2/src/render/xhr'; + +import {isBlank} from 'angular2/src/facade/lang'; +import {PromiseWrapper, Promise} from 'angular2/src/facade/async'; +import {Map, MapWrapper} from 'angular2/src/facade/collection'; import {DOM} from 'angular2/src/dom/dom_adapter'; @@ -24,10 +31,13 @@ export function main() { var strategy; describe('NativeShadowDomStrategy', () => { + var xhr; beforeEach(() => { var urlResolver = new UrlResolver(); var styleUrlResolver = new StyleUrlResolver(urlResolver); - strategy = new NativeShadowDomStrategy(styleUrlResolver); + xhr = new FakeXHR(); + var styleInliner = new StyleInliner(xhr, styleUrlResolver, urlResolver); + strategy = new NativeShadowDomStrategy(styleInliner, styleUrlResolver); }); if (DOM.supportsNativeShadowDOM()) { @@ -44,10 +54,39 @@ export function main() { .toHaveText(".foo {" + "background-image: url('http://base/img.jpg');" + "}"); }); - it('should not inline import rules', () => { - var styleElement = el(''); - strategy.processStyleElement('someComponent', 'http://base', styleElement); - expect(styleElement).toHaveText("@import 'http://base/other.css';"); - }); + it('should inline @import rules', inject([AsyncTestCompleter], (async) => { + xhr.reply('http://base/one.css', '.one {}'); + + var styleElement = el(''); + var stylePromise = + strategy.processStyleElement('someComponent', 'http://base', styleElement); + expect(stylePromise).toBePromise(); + + stylePromise.then((_) => { + expect(styleElement).toHaveText('.one {}\n'); + async.done(); + }); + })); + }); } + +class FakeXHR extends XHR { + _responses: Map; + + constructor() { + super(); + this._responses = MapWrapper.create(); + } + + get(url: string): Promise { + var response = MapWrapper.get(this._responses, url); + if (isBlank(response)) { + return PromiseWrapper.reject('xhr error', null); + } + + return PromiseWrapper.resolve(response); + } + + reply(url: string, response: string) { MapWrapper.set(this._responses, url, response); } +} diff --git a/modules/angular2/test/render/dom/shadow_dom_emulation_integration_spec.ts b/modules/angular2/test/render/dom/shadow_dom_emulation_integration_spec.ts index 658acf3d9f..ef4dcd81d3 100644 --- a/modules/angular2/test/render/dom/shadow_dom_emulation_integration_spec.ts +++ b/modules/angular2/test/render/dom/shadow_dom_emulation_integration_spec.ts @@ -43,17 +43,17 @@ export function main() { .toFactory((styleInliner, styleUrlResolver) => new EmulatedScopedShadowDomStrategy( styleInliner, styleUrlResolver, styleHost), [StyleInliner, StyleUrlResolver]), - "unscoped": bind(ShadowDomStrategy) - .toFactory((styleUrlResolver) => new EmulatedUnscopedShadowDomStrategy( - styleUrlResolver, styleHost), - [StyleUrlResolver]) + "unscoped": bind(ShadowDomStrategy).toFactory( + (styleInliner, styleUrlResolver) => new EmulatedUnscopedShadowDomStrategy( + styleInliner, styleUrlResolver, null), [StyleInliner, StyleUrlResolver]) }; if (DOM.supportsNativeShadowDOM()) { StringMapWrapper.set( strategies, "native", bind(ShadowDomStrategy) - .toFactory((styleUrlResolver) => new NativeShadowDomStrategy(styleUrlResolver), - [StyleUrlResolver])); + .toFactory((styleInliner, styleUrlResolver) => + new NativeShadowDomStrategy(styleInliner, styleUrlResolver), + [StyleInliner, StyleUrlResolver])); } beforeEach(() => { styleHost = el('
'); }); diff --git a/modules/benchmarks/src/compiler/compiler_benchmark.ts b/modules/benchmarks/src/compiler/compiler_benchmark.ts index 5ff4b5e038..bc176a6a08 100644 --- a/modules/benchmarks/src/compiler/compiler_benchmark.ts +++ b/modules/benchmarks/src/compiler/compiler_benchmark.ts @@ -17,6 +17,7 @@ import {TemplateLoader} from 'angular2/src/render/dom/compiler/template_loader'; import {TemplateResolver} from 'angular2/src/core/compiler/template_resolver'; import {UrlResolver} from 'angular2/src/services/url_resolver'; import {StyleUrlResolver} from 'angular2/src/render/dom/shadow_dom/style_url_resolver'; +import {StyleInliner} from 'angular2/src/render/dom/shadow_dom/style_inliner'; import {ComponentUrlMapper} from 'angular2/src/core/compiler/component_url_mapper'; import {reflector} from 'angular2/src/reflection/reflection'; @@ -37,7 +38,8 @@ export function main() { count, [BenchmarkComponentNoBindings, BenchmarkComponentWithBindings]); var urlResolver = new UrlResolver(); var styleUrlResolver = new StyleUrlResolver(urlResolver); - var shadowDomStrategy = new NativeShadowDomStrategy(styleUrlResolver); + var styleInliner = new StyleInliner(null, styleUrlResolver, urlResolver); + var shadowDomStrategy = new NativeShadowDomStrategy(styleInliner, styleUrlResolver); var renderCompiler = new rc.DefaultDomCompiler(new Parser(new Lexer()), shadowDomStrategy, new TemplateLoader(null, urlResolver)); var compiler =