Skip to content

Commit

Permalink
refactor(core): move server-only DomAdapter methods into ServerRender…
Browse files Browse the repository at this point in the history
…er (angular#32408)

PR Close angular#32408
  • Loading branch information
kara authored and mhevery committed Sep 3, 2019
1 parent 1ed3531 commit 970b58b
Show file tree
Hide file tree
Showing 16 changed files with 60 additions and 122 deletions.
4 changes: 2 additions & 2 deletions integration/_payload-limits.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@
"master": {
"uncompressed": {
"bundle": "TODO(i): temporarily increase the payload size limit from 105779 - this is due to a closure issue related to ESM reexports that still needs to be investigated",
"bundle": 179825
"bundle": 177447
}
}
}
}
}
27 changes: 6 additions & 21 deletions packages/common/src/dom_adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,33 +43,17 @@ export abstract class DomAdapter {
abstract querySelectorAll(el: any, selector: string): any[];
abstract remove(el: any): Node;
abstract getAttribute(element: any, attribute: string): string|null;

// Used by platform-server
abstract setProperty(el: Element, name: string, value: any): any;
abstract querySelector(el: any, selector: string): any;
abstract nextSibling(el: any): Node|null;
abstract parentElement(el: any): Node|null;
abstract clearNodes(el: any): any;
abstract appendChild(el: any, node: any): any;
abstract removeChild(el: any, node: any): any;
abstract insertBefore(parent: any, ref: any, node: any): any;
abstract setText(el: any, value: string): any;
abstract createComment(text: string): any;
abstract createElement(tagName: any, doc?: any): HTMLElement;
abstract createElementNS(ns: string, tagName: string, doc?: any): Element;
abstract createTextNode(text: string, doc?: any): Text;
abstract setAttribute(element: any, name: string, value: string): any;
abstract getElementsByTagName(element: any, name: string): HTMLElement[];
abstract addClass(element: any, className: string): any;
abstract removeClass(element: any, className: string): any;
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;
abstract setAttribute(element: any, name: string, value: string): any;
abstract setAttributeNS(element: any, ns: string, name: string, value: string): any;
abstract removeAttribute(element: any, attribute: string): any;
abstract removeAttributeNS(element: any, ns: string, attribute: string): any;
abstract createHtmlDocument(): HTMLDocument;
abstract getDefaultDocument(): Document;

// Used by Title
abstract getTitle(doc: Document): string;
Expand All @@ -80,6 +64,7 @@ export abstract class DomAdapter {
abstract isElementNode(node: any): boolean;

// Used by Testability
abstract parentElement(el: any): Node|null;
abstract isShadowRoot(node: any): boolean;
abstract getHost(el: any): any;

Expand Down
2 changes: 1 addition & 1 deletion packages/common/test/directives/ng_if_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ import {expect} from '@angular/platform-browser/testing/src/matchers';
fixture.detectChanges();
let els = fixture.debugElement.queryAll(By.css('span'));
expect(els.length).toEqual(1);
getDOM().addClass(els[0].nativeElement, 'marker');
els[0].nativeElement.classList.add('marker');
expect(fixture.nativeElement).toHaveText('hello');

getComponent().numberCondition = 2;
Expand Down
6 changes: 3 additions & 3 deletions packages/common/test/directives/non_bindable_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,23 +37,23 @@ import {expect} from '@angular/platform-browser/testing/src/matchers';

// We must use getDOM().querySelector instead of fixture.query here
// since the elements inside are not compiled.
const span = getDOM().querySelector(fixture.nativeElement, '#child');
const span = fixture.nativeElement.querySelector('#child');
expect(hasClass(span, 'compiled')).toBeFalsy();
}));

it('should trigger directives on the same node', async(() => {
const template = '<div><span id=child ngNonBindable test-dec>{{text}}</span></div>';
const fixture = createTestComponent(template);
fixture.detectChanges();
const span = getDOM().querySelector(fixture.nativeElement, '#child');
const span = fixture.nativeElement.querySelector('#child');
expect(hasClass(span, 'compiled')).toBeTruthy();
}));
});
}

@Directive({selector: '[test-dec]'})
class TestDirective {
constructor(el: ElementRef) { getDOM().addClass(el.nativeElement, 'compiled'); }
constructor(el: ElementRef) { el.nativeElement.classList.add('compiled'); }
}

@Component({selector: 'test-cmp', template: ''})
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/animation/animation_integration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1589,7 +1589,7 @@ const DEFAULT_COMPONENT_ID = '1';
engine.flush();
resetLog();

const element = getDOM().querySelector(fixture.nativeElement, '.ng-if');
const element = fixture.nativeElement.querySelector('.ng-if');
assertHasParent(element, true);

cmp.exp = false;
Expand Down
6 changes: 3 additions & 3 deletions packages/core/test/dom/dom_adapter_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {isTextNode} from '@angular/platform-browser/testing/src/browser_util';
});

it('should be able to create text nodes and use them with the other APIs', () => {
const t = getDOM().createTextNode('hello');
const t = getDOM().getDefaultDocument().createTextNode('hello');
expect(isTextNode(t)).toBe(true);
const d = getDOM().createElement('div');
getDOM().appendChild(d, t);
Expand Down Expand Up @@ -71,7 +71,7 @@ import {isTextNode} from '@angular/platform-browser/testing/src/browser_util';
getDOM().appendChild(headEl, baseEl);

const baseHref = getDOM().getBaseHref(defaultDoc);
getDOM().removeChild(headEl, baseEl);
headEl.removeChild(baseEl);
getDOM().resetBaseElement();

expect(baseHref).toEqual('/drop/bass/connon/');
Expand All @@ -84,7 +84,7 @@ import {isTextNode} from '@angular/platform-browser/testing/src/browser_util';
getDOM().appendChild(headEl, baseEl);

const baseHref = getDOM().getBaseHref(defaultDoc) !;
getDOM().removeChild(headEl, baseEl);
headEl.removeChild(baseEl);
getDOM().resetBaseElement();

expect(baseHref.endsWith('/base')).toBe(true);
Expand Down
12 changes: 4 additions & 8 deletions packages/core/test/linker/integration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1694,7 +1694,7 @@ function declareTests(config?: {useJit: boolean}) {
fixture.componentInstance.ctxProp = 'TITLE';
fixture.detectChanges();

const el = getDOM().querySelector(fixture.nativeElement, 'span');
const el = fixture.nativeElement.querySelector('span');
expect(el.title).toBeFalsy();
});

Expand All @@ -1707,7 +1707,7 @@ function declareTests(config?: {useJit: boolean}) {
fixture.componentInstance.ctxProp = 'TITLE';
fixture.detectChanges();

const el = getDOM().querySelector(fixture.nativeElement, 'span');
const el = fixture.nativeElement.querySelector('span');
expect(getDOM().getProperty(el, 'title')).toEqual('TITLE');
});
});
Expand Down Expand Up @@ -2685,16 +2685,12 @@ class ComponentWithoutView {

@Directive({selector: '[no-duplicate]'})
class DuplicateDir {
constructor(elRef: ElementRef) {
getDOM().setText(elRef.nativeElement, elRef.nativeElement.textContent + 'noduplicate');
}
constructor(elRef: ElementRef) { elRef.nativeElement.textContent += 'noduplicate'; }
}

@Directive({selector: '[no-duplicate]'})
class OtherDuplicateDir {
constructor(elRef: ElementRef) {
getDOM().setText(elRef.nativeElement, elRef.nativeElement.textContent + 'othernoduplicate');
}
constructor(elRef: ElementRef) { elRef.nativeElement.textContent += 'othernoduplicate'; }
}

@Directive({selector: 'directive-throwing-error'})
Expand Down
11 changes: 6 additions & 5 deletions packages/core/test/linker/projection_integration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,9 @@ describe('projection', () => {
componentFactoryResolver.resolveComponentFactory(MultipleContentTagsComponent);
expect(componentFactory.ngContentSelectors).toEqual(['h1', '*']);

const nodeOne = getDOM().createTextNode('one');
const nodeTwo = getDOM().createTextNode('two');

const nodeOne = getDOM().getDefaultDocument().createTextNode('one');
const nodeTwo = getDOM().getDefaultDocument().createTextNode('two');
const component = componentFactory.create(injector, [[nodeOne], [nodeTwo]]);
expect(component.location.nativeElement).toHaveText('(one, two)');
});
Expand Down Expand Up @@ -175,9 +176,9 @@ describe('projection', () => {
componentFactoryResolver.resolveComponentFactory(MultipleContentTagsComponent);
expect(componentFactory.ngContentSelectors).toEqual(['h1', '*', 'h2']);

const nodeOne = getDOM().createTextNode('one');
const nodeTwo = getDOM().createTextNode('two');
const nodeThree = getDOM().createTextNode('three');
const nodeOne = getDOM().getDefaultDocument().createTextNode('one');
const nodeTwo = getDOM().getDefaultDocument().createTextNode('two');
const nodeThree = getDOM().getDefaultDocument().createTextNode('three');
const component = componentFactory.create(injector, [[nodeOne], [nodeTwo], [nodeThree]]);
component.changeDetectorRef.detectChanges();
expect(component.location.nativeElement.textContent.trim()).toBe('1one 2two 3three');
Expand Down
29 changes: 0 additions & 29 deletions packages/platform-browser/src/browser/browser_adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ const nodeContains: (this: Node, other: Node) => boolean = (() => {
/* tslint:disable:requireParameterType no-console */
export class BrowserDomAdapter extends GenericBrowserDomAdapter {
static makeCurrent() { setRootDomAdapter(new BrowserDomAdapter()); }
setProperty(el: Node, name: string, value: any) { (<any>el)[name] = value; }
getProperty(el: Node, name: string): any { return (<any>el)[name]; }

log(error: string): void {
Expand All @@ -95,7 +94,6 @@ export class BrowserDomAdapter extends GenericBrowserDomAdapter {
}
}

querySelector(el: HTMLElement, selector: string): any { return el.querySelector(selector); }
querySelectorAll(el: any, selector: string): any[] { return el.querySelectorAll(selector); }
onAndCancel(el: Node, evt: any, listener: any): Function {
el.addEventListener(evt, listener, false);
Expand All @@ -104,43 +102,23 @@ export class BrowserDomAdapter extends GenericBrowserDomAdapter {
return () => { el.removeEventListener(evt, listener, false); };
}
dispatchEvent(el: Node, evt: any) { el.dispatchEvent(evt); }
nextSibling(el: Node): Node|null { return el.nextSibling; }
parentElement(el: Node): Node|null { return el.parentNode; }
clearNodes(el: Node) {
while (el.firstChild) {
el.removeChild(el.firstChild);
}
}
appendChild(el: Node, node: Node) { el.appendChild(node); }
removeChild(el: Node, node: Node) { el.removeChild(node); }
remove(node: Node): Node {
if (node.parentNode) {
node.parentNode.removeChild(node);
}
return node;
}
insertBefore(parent: Node, ref: Node, node: Node) { parent.insertBefore(node, ref); }
setText(el: Node, value: string) { el.textContent = value; }
getValue(el: any): string { return el.value; }
createComment(text: string): Comment { return this.getDefaultDocument().createComment(text); }
createElement(tagName: string, doc?: Document): HTMLElement {
doc = doc || this.getDefaultDocument();
return doc.createElement(tagName);
}
createElementNS(ns: string, tagName: string, doc?: Document): Element {
doc = doc || this.getDefaultDocument();
return doc.createElementNS(ns, tagName);
}
createTextNode(text: string, doc?: Document): Text {
doc = doc || this.getDefaultDocument();
return doc.createTextNode(text);
}
getHost(el: HTMLElement): HTMLElement { return (<any>el).host; }
getElementsByTagName(element: any, name: string): HTMLElement[] {
return element.getElementsByTagName(name);
}
addClass(element: any, className: string) { element.classList.add(className); }
removeClass(element: any, className: string) { element.classList.remove(className); }
setStyle(element: any, styleName: string, styleValue: string) {
element.style[styleName] = styleValue;
}
Expand All @@ -156,13 +134,6 @@ export class BrowserDomAdapter extends GenericBrowserDomAdapter {
return element.getAttribute(attribute);
}
setAttribute(element: Element, name: string, value: string) { element.setAttribute(name, value); }
setAttributeNS(element: Element, ns: string, name: string, value: string) {
element.setAttributeNS(ns, name, value);
}
removeAttribute(element: Element, attribute: string) { element.removeAttribute(attribute); }
removeAttributeNS(element: Element, ns: string, name: string) {
element.removeAttributeNS(ns, name);
}

createHtmlDocument(): HTMLDocument {
return document.implementation.createHTMLDocument('fakeTitle');
Expand Down
2 changes: 1 addition & 1 deletion packages/platform-browser/src/browser/meta.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export class Meta {

getTag(attrSelector: string): HTMLMetaElement|null {
if (!attrSelector) return null;
return this._dom.querySelector(this._doc, `meta[${attrSelector}]`) || null;
return this._doc.querySelector(`meta[${attrSelector}]`) || null;
}

getTags(attrSelector: string): HTMLMetaElement[] {
Expand Down
2 changes: 1 addition & 1 deletion packages/platform-browser/test/browser/bootstrap_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ function bootstrap(
getDOM().appendChild(doc.body, el);
getDOM().appendChild(doc.body, el2);
getDOM().appendChild(el, lightDom);
getDOM().setText(lightDom, 'loading');
lightDom.textContent = 'loading';
}));

afterEach(destroyPlatform);
Expand Down
14 changes: 1 addition & 13 deletions packages/platform-server/src/domino_adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,18 +86,6 @@ export class DominoAdapter extends BrowserDomAdapter {
return (<any>el)[name];
}

setProperty(el: Element, name: string, value: any) {
if (name === 'href') {
// Even though the server renderer reflects any properties to attributes
// map 'href' to attribute just to handle when setProperty is directly called.
this.setAttribute(el, 'href', value);
} else if (name === 'innerText') {
// Domino does not support innerText. Just map it to textContent.
el.textContent = value;
}
(<any>el)[name] = value;
}

getGlobalEventTarget(doc: Document, target: string): EventTarget|null {
if (target === 'window') {
return doc.defaultView;
Expand All @@ -112,7 +100,7 @@ export class DominoAdapter extends BrowserDomAdapter {
}

getBaseHref(doc: Document): string {
const base = this.querySelector(doc.documentElement !, 'base');
const base = doc.documentElement !.querySelector('base');
let href = '';
if (base) {
href = base.getAttribute('href') !;
Expand Down
Loading

0 comments on commit 970b58b

Please sign in to comment.