refactor(core): move DomAdapter style methods to ServerRenderer (angu…

…lar#32408) PR Close angular#32408
rmshea · Sep 3, 2019 · c207ad8 · c207ad8
1 parent 970b58b
commit c207ad8
Show file tree

Hide file tree

Showing 10 changed files with 46 additions and 100 deletions.
diff --git a/packages/common/src/dom_adapter.ts b/packages/common/src/dom_adapter.ts
@@ -50,11 +50,6 @@ export abstract class DomAdapter {
   abstract createHtmlDocument(): HTMLDocument;
   abstract getDefaultDocument(): Document;
 
-  // Used by platform-server
-  abstract getStyle(element: any, styleName: string): any;
-  abstract setStyle(element: any, styleName: string, styleValue: string): any;
-  abstract removeStyle(element: any, styleName: string): any;
-
   // Used by Title
   abstract getTitle(doc: Document): string;
   abstract setTitle(doc: Document, newTitle: string): any;

diff --git a/packages/core/test/dom/dom_adapter_spec.ts b/packages/core/test/dom/dom_adapter_spec.ts
@@ -36,27 +36,6 @@ import {isTextNode} from '@angular/platform-browser/testing/src/browser_util';
       expect(() => getDOM().remove(d)).not.toThrow();
     });
 
-    it('should parse styles with urls correctly', () => {
-      const d = getDOM().createElement('div');
-      getDOM().setStyle(d, 'background-url', 'url(http://test.com/bg.jpg)');
-      expect(getDOM().getStyle(d, 'background-url')).toBe('url(http://test.com/bg.jpg)');
-    });
-
-    // Test for regression caused by angular/angular#22536
-    it('should parse styles correctly following the spec', () => {
-      const d = getDOM().createElement('div');
-      getDOM().setStyle(d, 'background-image', 'url("paper.gif")');
-      expect(d.style.backgroundImage).toBe('url("paper.gif")');
-      expect(d.style.getPropertyValue('background-image')).toBe('url("paper.gif")');
-      expect(getDOM().getStyle(d, 'background-image')).toBe('url("paper.gif")');
-    });
-
-    it('should parse camel-case styles correctly', () => {
-      const d = getDOM().createElement('div');
-      getDOM().setStyle(d, 'marginRight', '10px');
-      expect(getDOM().getStyle(d, 'margin-right')).toBe('10px');
-    });
-
     if (getDOM().supportsDOMEvents()) {
       describe('getBaseHref', () => {
         beforeEach(() => getDOM().resetBaseElement());

diff --git a/packages/core/test/linker/integration_spec.ts b/packages/core/test/linker/integration_spec.ts
@@ -147,13 +147,11 @@ function declareTests(config?: {useJit: boolean}) {
 
         fixture.componentInstance.ctxProp = '10';
         fixture.detectChanges();
-        expect(getDOM().getStyle(fixture.debugElement.children[0].nativeElement, 'height'))
-            .toEqual('10px');
+        expect(fixture.debugElement.children[0].nativeElement.style['height']).toEqual('10px');
 
         fixture.componentInstance.ctxProp = null !;
         fixture.detectChanges();
-        expect(getDOM().getStyle(fixture.debugElement.children[0].nativeElement, 'height'))
-            .toEqual('');
+        expect(fixture.debugElement.children[0].nativeElement.style['height']).toEqual('');
       });
 
       it('should consume binding to property names where attr name and property name do not match',

diff --git a/packages/core/test/linker/security_integration_spec.ts b/packages/core/test/linker/security_integration_spec.ts
@@ -247,12 +247,12 @@ function declareTests(config?: {useJit: boolean}) {
         fixture.detectChanges();
         // In some browsers, this will contain the full background specification, not just
         // the color.
-        expect(getDOM().getStyle(e, 'background')).toMatch(/red.*/);
+        expect(e.style['background']).toMatch(/red.*/);
 
         ci.ctxProp = 'url(javascript:evil())';
         fixture.detectChanges();
         // Updated value gets rejected, no value change.
-        expect(getDOM().getStyle(e, 'background')).not.toContain('javascript');
+        expect(e.style['background']).not.toContain('javascript');
       });
 
       modifiedInIvy('Unknown property error thrown during update mode, not creation mode')

diff --git a/packages/core/test/view/element_spec.ts b/packages/core/test/view/element_spec.ts
@@ -164,8 +164,8 @@ const removeEventListener = '__zone_symbol__removeEventListener' as 'removeEvent
           Services.checkAndUpdateView(view);
 
           const el = rootNodes[0];
-          expect(getDOM().getStyle(el, 'width')).toBe('10px');
-          expect(getDOM().getStyle(el, 'color')).toBe('red');
+          expect(el.style['width']).toBe('10px');
+          expect(el.style['color']).toBe('red');
         });
       });
     });

diff --git a/packages/platform-browser/src/browser/browser_adapter.ts b/packages/platform-browser/src/browser/browser_adapter.ts
@@ -119,16 +119,6 @@ export class BrowserDomAdapter extends GenericBrowserDomAdapter {
   getElementsByTagName(element: any, name: string): HTMLElement[] {
     return element.getElementsByTagName(name);
   }
-  setStyle(element: any, styleName: string, styleValue: string) {
-    element.style[styleName] = styleValue;
-  }
-  removeStyle(element: any, stylename: string) {
-    // IE requires '' instead of null
-    // see https://github.com/angular/angular/issues/7916
-    element.style[stylename] = '';
-  }
-
-  getStyle(element: any, stylename: string): string { return element.style[stylename]; }
 
   getAttribute(element: Element, attribute: string): string|null {
     return element.getAttribute(attribute);

diff --git a/packages/platform-server/src/domino_adapter.ts b/packages/platform-server/src/domino_adapter.ts
@@ -109,55 +109,6 @@ export class DominoAdapter extends BrowserDomAdapter {
     return href;
   }
 
-  /** @internal */
-  _readStyleAttribute(element: any): {[name: string]: string} {
-    const styleMap: {[name: string]: string} = {};
-    const styleAttribute = element.getAttribute('style');
-    if (styleAttribute) {
-      const styleList = styleAttribute.split(/;+/g);
-      for (let i = 0; i < styleList.length; i++) {
-        const style = styleList[i].trim();
-        if (style.length > 0) {
-          const colonIndex = style.indexOf(':');
-          if (colonIndex === -1) {
-            throw new Error(`Invalid CSS style: ${style}`);
-          }
-          const name = style.substr(0, colonIndex).trim();
-          styleMap[name] = style.substr(colonIndex + 1).trim();
-        }
-      }
-    }
-    return styleMap;
-  }
-  /** @internal */
-  _writeStyleAttribute(element: any, styleMap: {[name: string]: string}) {
-    let styleAttrValue = '';
-    for (const key in styleMap) {
-      const newValue = styleMap[key];
-      if (newValue) {
-        styleAttrValue += key + ':' + styleMap[key] + ';';
-      }
-    }
-    element.setAttribute('style', styleAttrValue);
-  }
-
-  setStyle(element: any, styleName: string, styleValue?: string|null) {
-    styleName = styleName.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase();
-    const styleMap = this._readStyleAttribute(element);
-    styleMap[styleName] = styleValue || '';
-    this._writeStyleAttribute(element, styleMap);
-  }
-  removeStyle(element: any, styleName: string) {
-    // IE requires '' instead of null
-    // see https://github.com/angular/angular/issues/7916
-    this.setStyle(element, styleName, '');
-  }
-
-  getStyle(element: any, styleName: string): string {
-    const styleMap = this._readStyleAttribute(element);
-    return styleMap[styleName] || '';
-  }
-
   dispatchEvent(el: Node, evt: any) {
     el.dispatchEvent(evt);
 

diff --git a/packages/platform-server/src/server_renderer.ts b/packages/platform-server/src/server_renderer.ts
@@ -143,11 +143,16 @@ class DefaultServerRenderer2 implements Renderer2 {
   removeClass(el: any, name: string): void { el.classList.remove(name); }
 
   setStyle(el: any, style: string, value: any, flags: RendererStyleFlags2): void {
-    getDOM().setStyle(el, style, value);
+    style = style.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase();
+    const styleMap = _readStyleAttribute(el);
+    styleMap[style] = value || '';
+    _writeStyleAttribute(el, styleMap);
   }
 
   removeStyle(el: any, style: string, flags: RendererStyleFlags2): void {
-    getDOM().removeStyle(el, style);
+    // IE requires '' instead of null
+    // see https://github.com/angular/angular/issues/7916
+    this.setStyle(el, style, '', flags);
   }
 
   // The value was validated already as a property binding, against the property name.
@@ -246,3 +251,34 @@ class EmulatedEncapsulationServerRenderer2 extends DefaultServerRenderer2 {
     return el;
   }
 }
+
+function _readStyleAttribute(element: any): {[name: string]: string} {
+  const styleMap: {[name: string]: string} = {};
+  const styleAttribute = element.getAttribute('style');
+  if (styleAttribute) {
+    const styleList = styleAttribute.split(/;+/g);
+    for (let i = 0; i < styleList.length; i++) {
+      const style = styleList[i].trim();
+      if (style.length > 0) {
+        const colonIndex = style.indexOf(':');
+        if (colonIndex === -1) {
+          throw new Error(`Invalid CSS style: ${style}`);
+        }
+        const name = style.substr(0, colonIndex).trim();
+        styleMap[name] = style.substr(colonIndex + 1).trim();
+      }
+    }
+  }
+  return styleMap;
+}
+
+function _writeStyleAttribute(element: any, styleMap: {[name: string]: string}) {
+  let styleAttrValue = '';
+  for (const key in styleMap) {
+    const newValue = styleMap[key];
+    if (newValue) {
+      styleAttrValue += key + ':' + styleMap[key] + ';';
+    }
+  }
+  element.setAttribute('style', styleAttrValue);
+}
diff --git a/packages/platform-webworker/src/web_workers/worker/worker_adapter.ts b/packages/platform-webworker/src/web_workers/worker/worker_adapter.ts
@@ -53,9 +53,6 @@ export class WorkerDomAdapter extends DomAdapter {
   createElement(tagName: any, doc?: any): HTMLElement { throw 'not implemented'; }
   getHost(el: any): any { throw 'not implemented'; }
   getElementsByTagName(element: any, name: string): HTMLElement[] { throw 'not implemented'; }
-  setStyle(element: any, styleName: string, styleValue: string) { throw 'not implemented'; }
-  removeStyle(element: any, styleName: string) { throw 'not implemented'; }
-  getStyle(element: any, styleName: string): string { throw 'not implemented'; }
   getAttribute(element: any, attribute: string): string { throw 'not implemented'; }
   setAttribute(element: any, name: string, value: string) { throw 'not implemented'; }
   createHtmlDocument(): HTMLDocument { throw 'not implemented'; }

diff --git a/packages/platform-webworker/test/web_workers/worker/renderer_v2_integration_spec.ts b/packages/platform-webworker/test/web_workers/worker/renderer_v2_integration_spec.ts
@@ -115,10 +115,10 @@ let lastCreatedRenderer: Renderer2;
                 expect(hasClass(el, 'a')).toBe(false);
 
                 lastCreatedRenderer.setStyle(workerEl, 'width', '10px');
-                expect(getDOM().getStyle(el, 'width')).toEqual('10px');
+                expect(el.style['width']).toEqual('10px');
 
                 lastCreatedRenderer.removeStyle(workerEl, 'width');
-                expect(getDOM().getStyle(el, 'width')).toEqual('');
+                expect(el.style['width']).toEqual('');
 
                 lastCreatedRenderer.setAttribute(workerEl, 'someattr', 'someValue');
                 expect(getDOM().getAttribute(el, 'someattr')).toEqual('someValue');