Skip to content

Commit

Permalink
refactor(core): move Meta methods that only have one version from Dom…
Browse files Browse the repository at this point in the history
…Adapter (angular#32408)

PR Close angular#32408
  • Loading branch information
kara authored and mhevery committed Sep 3, 2019
1 parent 1a7c797 commit 89434e0
Show file tree
Hide file tree
Showing 28 changed files with 88 additions and 111 deletions.
6 changes: 3 additions & 3 deletions aio/scripts/_payload-limits.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
"uncompressed": {
"runtime-es5": 3042,
"runtime-es2015": 3048,
"main-es5": 499085,
"main-es2015": 438296,
"main-es5": 493330,
"main-es2015": 435296,
"polyfills-es5": 131024,
"polyfills-es2015": 52433
}
Expand All @@ -28,7 +28,7 @@
"uncompressed": {
"runtime-es5": 2932,
"runtime-es2015": 2938,
"main-es5": 552068,
"main-es5": 550854,
"main-es2015": 493320,
"polyfills-es5": 131024,
"polyfills-es2015": 52433
Expand Down
4 changes: 2 additions & 2 deletions integration/_payload-limits.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"master": {
"uncompressed": {
"runtime": 1497,
"main": 158490,
"main": 157490,
"polyfills": 45399
}
}
Expand All @@ -21,7 +21,7 @@
"master": {
"uncompressed": {
"runtime": 1440,
"main": 125882,
"main": 123904,
"polyfills": 45340
}
}
Expand Down
5 changes: 0 additions & 5 deletions packages/common/src/dom_adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,8 @@ export abstract class DomAdapter {
abstract logGroupEnd(): any;

// Used by Meta
abstract querySelectorAll(el: any, selector: string): any[];
abstract remove(el: any): Node;
abstract getAttribute(element: any, attribute: string): string|null;
abstract appendChild(el: any, node: any): any;
abstract createElement(tagName: any, doc?: any): HTMLElement;
abstract setAttribute(element: any, name: string, value: string): any;
abstract getElementsByTagName(element: any, name: string): HTMLElement[];
abstract createHtmlDocument(): HTMLDocument;
abstract getDefaultDocument(): Document;

Expand Down
2 changes: 1 addition & 1 deletion packages/compiler/test/selector/selector_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ import {el} from '@angular/platform-browser/testing/src/browser_util';

const elementSelector = new CssSelector();
const element = el('<div attr></div>');
const empty = getDOM().getAttribute(element, 'attr') !;
const empty = element.getAttribute('attr') !;
elementSelector.addAttribute('some-decor', empty);
matcher.match(elementSelector, selectableCollector);
expect(matched).toEqual([s1[0], 1]);
Expand Down
4 changes: 2 additions & 2 deletions packages/core/test/application_ref_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ class SomeComponent {
const doc = TestBed.get(DOCUMENT);
const rootEl =
<HTMLElement>getContent(createTemplate(`<${selector}></${selector}>`)).firstChild;
const oldRoots = getDOM().querySelectorAll(doc, selector);
const oldRoots = doc.querySelectorAll(selector);
for (let i = 0; i < oldRoots.length; i++) {
getDOM().remove(oldRoots[i]);
}
getDOM().appendChild(doc.body, rootEl);
doc.body.appendChild(rootEl);
}

type CreateModuleOptions =
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/debug/debug_node_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -883,7 +883,7 @@ class TestCmptWithPropBindings {

// Move the content element outside the component
// so that it can't be reached via querySelector.
getDOM().appendChild(parent, content);
parent.appendChild(content);

expect(fixture.debugElement.query(By.css('.content'))).toBeTruthy();

Expand Down
12 changes: 6 additions & 6 deletions packages/core/test/dom/dom_adapter_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ import {isTextNode} from '@angular/platform-browser/testing/src/browser_util';
const t = getDOM().getDefaultDocument().createTextNode('hello');
expect(isTextNode(t)).toBe(true);
const d = getDOM().createElement('div');
getDOM().appendChild(d, t);
d.appendChild(t);
expect(d.innerHTML).toEqual('hello');
});

it('should set className via the class attribute', () => {
const d = getDOM().createElement('div');
getDOM().setAttribute(d, 'class', 'class1');
d.setAttribute('class', 'class1');
expect(d.className).toEqual('class1');
});

Expand All @@ -45,9 +45,9 @@ import {isTextNode} from '@angular/platform-browser/testing/src/browser_util';

it('should return the value of the base element', () => {
const baseEl = getDOM().createElement('base');
getDOM().setAttribute(baseEl, 'href', '/drop/bass/connon/');
baseEl.setAttribute('href', '/drop/bass/connon/');
const headEl = defaultDoc.head;
getDOM().appendChild(headEl, baseEl);
headEl.appendChild(baseEl);

const baseHref = getDOM().getBaseHref(defaultDoc);
headEl.removeChild(baseEl);
Expand All @@ -58,9 +58,9 @@ import {isTextNode} from '@angular/platform-browser/testing/src/browser_util';

it('should return a relative url', () => {
const baseEl = getDOM().createElement('base');
getDOM().setAttribute(baseEl, 'href', 'base');
baseEl.setAttribute('href', 'base');
const headEl = defaultDoc.head;
getDOM().appendChild(headEl, baseEl);
headEl.appendChild(baseEl);

const baseHref = getDOM().getBaseHref(defaultDoc) !;
headEl.removeChild(baseEl);
Expand Down
17 changes: 8 additions & 9 deletions packages/core/test/linker/integration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,12 @@ function declareTests(config?: {useJit: boolean}) {

fixture.componentInstance.ctxProp = 'Initial aria label';
fixture.detectChanges();
expect(getDOM().getAttribute(fixture.debugElement.children[0].nativeElement, 'aria-label'))
expect(fixture.debugElement.children[0].nativeElement.getAttribute('aria-label'))
.toEqual('Initial aria label');

fixture.componentInstance.ctxProp = 'Changed aria label';
fixture.detectChanges();
expect(getDOM().getAttribute(fixture.debugElement.children[0].nativeElement, 'aria-label'))
expect(fixture.debugElement.children[0].nativeElement.getAttribute('aria-label'))
.toEqual('Changed aria label');
});

Expand All @@ -131,8 +131,7 @@ function declareTests(config?: {useJit: boolean}) {

fixture.componentInstance.ctxProp = 'bar';
fixture.detectChanges();
expect(getDOM().getAttribute(fixture.debugElement.children[0].nativeElement, 'foo'))
.toEqual('bar');
expect(fixture.debugElement.children[0].nativeElement.getAttribute('foo')).toEqual('bar');

fixture.componentInstance.ctxProp = null !;
fixture.detectChanges();
Expand Down Expand Up @@ -887,7 +886,7 @@ function declareTests(config?: {useJit: boolean}) {

fixture.detectChanges();

expect(getDOM().getAttribute(fixture.debugElement.nativeElement, 'role')).toEqual('button');
expect(fixture.debugElement.nativeElement.getAttribute('role')).toEqual('button');
});

it('should support updating host element via hostAttributes on host elements', () => {
Expand All @@ -898,7 +897,7 @@ function declareTests(config?: {useJit: boolean}) {

fixture.detectChanges();

expect(getDOM().getAttribute(fixture.debugElement.children[0].nativeElement, 'role'))
expect(fixture.debugElement.children[0].nativeElement.getAttribute('role'))
.toEqual('button');
});

Expand Down Expand Up @@ -1404,7 +1403,7 @@ function declareTests(config?: {useJit: boolean}) {
TestBed.overrideComponent(MyComp, {set: {template}});
const fixture = TestBed.createComponent(MyComp);

expect(getDOM().querySelectorAll(fixture.nativeElement, 'script').length).toEqual(0);
expect(fixture.nativeElement.querySelectorAll('script').length).toEqual(0);
});

it('should throw when using directives without selector in NgModule declarations', () => {
Expand Down Expand Up @@ -2139,7 +2138,7 @@ class SimpleImperativeViewComponent {

constructor(self: ElementRef) {
const hostElement = self.nativeElement;
getDOM().appendChild(hostElement, el('hello imp view'));
hostElement.appendChild(el('hello imp view'));
}
}

Expand Down Expand Up @@ -2663,7 +2662,7 @@ class SomeImperativeViewport {
this.view = this.vc.createEmbeddedView(this.templateRef);
const nodes = this.view.rootNodes;
for (let i = 0; i < nodes.length; i++) {
getDOM().appendChild(this.anchor, nodes[i]);
this.anchor.appendChild(nodes[i]);
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions packages/core/test/linker/projection_integration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -524,8 +524,8 @@ describe('projection', () => {
const mainEl = main.nativeElement;
const div1 = mainEl.firstChild;
const div2 = getDOM().createElement('div');
getDOM().setAttribute(div2, 'class', 'redStyle');
getDOM().appendChild(mainEl, div2);
div2.setAttribute('class', 'redStyle');
mainEl.appendChild(div2);
expect(getComputedStyle(div1).color).toEqual('rgb(255, 0, 0)');
expect(getComputedStyle(div2).color).toEqual('rgb(255, 0, 0)');
});
Expand All @@ -544,7 +544,7 @@ describe('projection', () => {
const mainEl = main.nativeElement;
const div1 = mainEl.firstChild;
const div2 = getDOM().createElement('div');
getDOM().appendChild(mainEl, div2);
mainEl.appendChild(div2);
expect(getComputedStyle(div1).color).toEqual('rgb(255, 0, 0)');
expect(getComputedStyle(div2).color).toEqual('rgb(0, 0, 0)');
});
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/linker/regression_integration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ function declareTestsUsingBootstrap() {
beforeEach(inject([DOCUMENT], (doc: any) => {
destroyPlatform();
const el = getDOM().createElement(COMP_SELECTOR, doc);
getDOM().appendChild(doc.body, el);
doc.body.appendChild(el);

logger = new MockConsole();
errorHandler = new ErrorHandler();
Expand Down
5 changes: 2 additions & 3 deletions packages/core/test/linker/security_integration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,13 +177,12 @@ function declareTests(config?: {useJit: boolean}) {
fixture.detectChanges();
// In the browser, reading href returns an absolute URL. On the server side,
// it just echoes back the property.
let value =
isAttribute ? getDOM().getAttribute(e, 'href') : getDOM().getProperty(e, 'href');
let value = isAttribute ? e.getAttribute('href') : getDOM().getProperty(e, 'href');
expect(value).toMatch(/.*\/?hello$/);

ci.ctxProp = 'javascript:alert(1)';
fixture.detectChanges();
value = isAttribute ? getDOM().getAttribute(e, 'href') : getDOM().getProperty(e, 'href');
value = isAttribute ? e.getAttribute('href') : getDOM().getProperty(e, 'href');
expect(value).toEqual('unsafe:javascript:alert(1)');
}

Expand Down
6 changes: 3 additions & 3 deletions packages/core/test/view/element_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const removeEventListener = '__zone_symbol__removeEventListener' as 'removeEvent
elementDef(0, NodeFlags.None, null, null, 0, 'div', [['title', 'a']]),
])).rootNodes;
expect(rootNodes.length).toBe(1);
expect(getDOM().getAttribute(rootNodes[0], 'title')).toBe('a');
expect(rootNodes[0].getAttribute('title')).toBe('a');
});

it('should add debug information to the renderer', () => {
Expand Down Expand Up @@ -114,8 +114,8 @@ const removeEventListener = '__zone_symbol__removeEventListener' as 'removeEvent
Services.checkAndUpdateView(view);

const el = rootNodes[0];
expect(getDOM().getAttribute(el, 'a1')).toBe('v1');
expect(getDOM().getAttribute(el, 'a2')).toBe('v2');
expect(el.getAttribute('a1')).toBe('v1');
expect(el.getAttribute('a2')).toBe('v2');
});
});
});
Expand Down
12 changes: 6 additions & 6 deletions packages/core/test/view/embedded_view_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ import {compViewDef, compViewDefFactory, createAndGetRootNodes, createEmbeddedVi
// 2 anchors + 2 elements
const rootChildren = rootNodes[0].childNodes;
expect(rootChildren.length).toBe(4);
expect(getDOM().getAttribute(rootChildren[1], 'name')).toBe('child0');
expect(getDOM().getAttribute(rootChildren[2], 'name')).toBe('child1');
expect(rootChildren[1].getAttribute('name')).toBe('child0');
expect(rootChildren[2].getAttribute('name')).toBe('child1');

rf.begin !();
detachEmbeddedView(viewContainerData, 1);
Expand Down Expand Up @@ -90,8 +90,8 @@ import {compViewDef, compViewDefFactory, createAndGetRootNodes, createEmbeddedVi
// 2 anchors + 2 elements
const rootChildren = rootNodes[0].childNodes;
expect(rootChildren.length).toBe(4);
expect(getDOM().getAttribute(rootChildren[1], 'name')).toBe('child1');
expect(getDOM().getAttribute(rootChildren[2], 'name')).toBe('child0');
expect(rootChildren[1].getAttribute('name')).toBe('child1');
expect(rootChildren[2].getAttribute('name')).toBe('child0');
});

it('should include embedded views in root nodes', () => {
Expand All @@ -107,8 +107,8 @@ import {compViewDef, compViewDefFactory, createAndGetRootNodes, createEmbeddedVi

const rootNodes = rootRenderNodes(parentView);
expect(rootNodes.length).toBe(3);
expect(getDOM().getAttribute(rootNodes[1], 'name')).toBe('child0');
expect(getDOM().getAttribute(rootNodes[2], 'name')).toBe('after');
expect(rootNodes[1].getAttribute('name')).toBe('child0');
expect(rootNodes[2].getAttribute('name')).toBe('after');
});

it('should dirty check embedded views', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/view/provider_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ import {ARG_TYPE_VALUES, checkNodeInlineOrDynamic, createRootView, createAndGetR
expect(instance.b).toBe('v2');

const el = rootNodes[0];
expect(getDOM().getAttribute(el, 'ng-reflect-a')).toBe('v1');
expect(el.getAttribute('ng-reflect-a')).toBe('v1');
});

});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ export class DOMTestComponentRenderer extends TestComponentRenderer {
const rootEl = <HTMLElement>getContent(template).firstChild;

// TODO(juliemr): can/should this be optional?
const oldRoots = getDOM().querySelectorAll(this._doc, '[id^=root]');
const oldRoots = this._doc.querySelectorAll('[id^=root]');
for (let i = 0; i < oldRoots.length; i++) {
getDOM().remove(oldRoots[i]);
}
getDOM().appendChild(this._doc.body, rootEl);
this._doc.body.appendChild(rootEl);
}
}

Expand Down
11 changes: 0 additions & 11 deletions packages/platform-browser/src/browser/browser_adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,13 @@ export class BrowserDomAdapter extends GenericBrowserDomAdapter {
}
}

querySelectorAll(el: any, selector: string): any[] { return el.querySelectorAll(selector); }
onAndCancel(el: Node, evt: any, listener: any): Function {
el.addEventListener(evt, listener, false);
// Needed to follow Dart's subscription semantic, until fix of
// https://code.google.com/p/dart/issues/detail?id=17406
return () => { el.removeEventListener(evt, listener, false); };
}
dispatchEvent(el: Node, evt: any) { el.dispatchEvent(evt); }
appendChild(el: Node, node: Node) { el.appendChild(node); }
remove(node: Node): Node {
if (node.parentNode) {
node.parentNode.removeChild(node);
Expand All @@ -70,15 +68,6 @@ export class BrowserDomAdapter extends GenericBrowserDomAdapter {
doc = doc || this.getDefaultDocument();
return doc.createElement(tagName);
}
getElementsByTagName(element: any, name: string): HTMLElement[] {
return element.getElementsByTagName(name);
}

getAttribute(element: Element, attribute: string): string|null {
return element.getAttribute(attribute);
}
setAttribute(element: Element, name: string, value: string) { element.setAttribute(name, value); }

createHtmlDocument(): HTMLDocument {
return document.implementation.createHTMLDocument('fakeTitle');
}
Expand Down
10 changes: 5 additions & 5 deletions packages/platform-browser/src/browser/meta.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export class Meta {

getTags(attrSelector: string): HTMLMetaElement[] {
if (!attrSelector) return [];
const list /*NodeList*/ = this._dom.querySelectorAll(this._doc, `meta[${attrSelector}]`);
const list /*NodeList*/ = this._doc.querySelectorAll(`meta[${attrSelector}]`);
return list ? [].slice.call(list) : [];
}

Expand Down Expand Up @@ -99,13 +99,13 @@ export class Meta {
}
const element: HTMLMetaElement = this._dom.createElement('meta') as HTMLMetaElement;
this._setMetaElementAttributes(meta, element);
const head = this._dom.getElementsByTagName(this._doc, 'head')[0];
this._dom.appendChild(head, element);
const head = this._doc.getElementsByTagName('head')[0];
head.appendChild(element);
return element;
}

private _setMetaElementAttributes(tag: MetaDefinition, el: HTMLMetaElement): HTMLMetaElement {
Object.keys(tag).forEach((prop: string) => this._dom.setAttribute(el, prop, tag[prop]));
Object.keys(tag).forEach((prop: string) => el.setAttribute(prop, tag[prop]));
return el;
}

Expand All @@ -115,6 +115,6 @@ export class Meta {
}

private _containsAttributes(tag: MetaDefinition, elem: HTMLMetaElement): boolean {
return Object.keys(tag).every((key: string) => this._dom.getAttribute(elem, key) === tag[key]);
return Object.keys(tag).every((key: string) => elem.getAttribute(key) === tag[key]);
}
}
2 changes: 1 addition & 1 deletion packages/platform-browser/src/browser/server-transition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export function appInitializerFactory(transitionId: string, document: any, injec
const dom = getDOM();
const styles: any[] =
Array.prototype.slice.apply(document.querySelectorAll(`style[ng-transition]`));
styles.filter(el => dom.getAttribute(el, 'ng-transition') === transitionId)
styles.filter(el => el.getAttribute('ng-transition') === transitionId)
.forEach(el => dom.remove(el));
});
};
Expand Down
Loading

0 comments on commit 89434e0

Please sign in to comment.