From 3c66b100dd6f05f53740f596c5eadb999c27c9c4 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 10 Mar 2021 08:20:40 +0100 Subject: [PATCH] perf(common): remove unused methods from DomAdapter (#41102) The `DomAdapter` is present in all Angular apps and its methods aren't tree shakeable. These changes remove the methods that either aren't being used anymore or were only used by our own tests. Note that these changes aren't breaking, because the adapter is an internal API. The following methods were removed: * `getProperty` - only used within our own tests. * `log` - Guaranteed to be defined on `console`. * `logGroup` and `logGroupEnd` - Only used in one place. It was in the DomAdapter for built-in null checking. * `logGroupEnd` - Only used in one place. It was placed in the DomAdapter for built in null checking. * `performanceNow` - Only used in one place that has to be invoked through the browser console. * `supportsCookies` - Unused. * `getCookie` - Unused. * `getLocation` and `getHistory` - Only used in one place which appears to have access to the DOM already, because it had direct accesses to `window`. Furthermore, even if this was being used in a non-browser context already, the `DominoAdapter` was set up to throw an error. The following APIs were changed to be more compact: * `supportsDOMEvents` - Changed to a readonly property. * `remove` - No longer returns the removed node. PR Close #41102 --- .../size-tracking/integration-payloads.json | 12 +-- packages/common/src/dom_adapter.ts | 19 +---- .../common/src/location/platform_location.ts | 4 +- packages/core/test/application_ref_spec.ts | 2 +- packages/core/test/dom/dom_adapter_spec.ts | 4 +- packages/core/test/linker/integration_spec.ts | 46 +++++------- .../linker/projection_integration_spec.ts | 2 +- packages/core/test/linker/query_list_spec.ts | 2 +- .../linker/regression_integration_spec.ts | 2 +- .../test/linker/security_integration_spec.ts | 32 +++----- packages/core/test/view/element_spec.ts | 4 +- packages/core/test/view/helper.ts | 2 +- .../src/browser/browser_adapter.ts | 74 ++----------------- .../src/browser/generic_browser_adapter.ts | 8 +- .../src/browser/tools/common_tools.ts | 12 ++- .../test/browser/bootstrap_spec.ts | 2 +- .../test/dom/events/event_manager_spec.ts | 10 +-- .../platform-server/src/domino_adapter.ts | 54 +------------- .../platform-server/test/integration_spec.ts | 2 +- .../test/server_styles_host_spec.ts | 2 +- packages/router/src/router_module.ts | 13 ++-- 21 files changed, 84 insertions(+), 224 deletions(-) diff --git a/goldens/size-tracking/integration-payloads.json b/goldens/size-tracking/integration-payloads.json index 442de79c01..42d259c003 100644 --- a/goldens/size-tracking/integration-payloads.json +++ b/goldens/size-tracking/integration-payloads.json @@ -3,7 +3,7 @@ "master": { "uncompressed": { "runtime-es2015": 1485, - "main-es2015": 138937, + "main-es2015": 138189, "polyfills-es2015": 36964 } } @@ -21,7 +21,7 @@ "master": { "uncompressed": { "runtime-es2015": 1485, - "main-es2015": 144899, + "main-es2015": 144151, "polyfills-es2015": 36964 } } @@ -30,7 +30,7 @@ "master": { "uncompressed": { "runtime-es2015": 1485, - "main-es2015": 137236, + "main-es2015": 136546, "polyfills-es2015": 37641 } } @@ -39,7 +39,7 @@ "master": { "uncompressed": { "runtime-es2015": 2285, - "main-es2015": 241085, + "main-es2015": 240352, "polyfills-es2015": 36975, "5-es2015": 753 } @@ -49,7 +49,7 @@ "master": { "uncompressed": { "runtime-es2015": 2289, - "main-es2015": 216925, + "main-es2015": 216267, "polyfills-es2015": 36723, "5-es2015": 781 } @@ -59,7 +59,7 @@ "master": { "uncompressed": { "runtime-es2015": 1485, - "main-es2015": 169047, + "main-es2015": 168534, "polyfills-es2015": 36975 } } diff --git a/packages/common/src/dom_adapter.ts b/packages/common/src/dom_adapter.ts index 0e2d832dbe..3b71475f85 100644 --- a/packages/common/src/dom_adapter.ts +++ b/packages/common/src/dom_adapter.ts @@ -31,16 +31,11 @@ export function setRootDomAdapter(adapter: DomAdapter) { */ export abstract class DomAdapter { // Needs Domino-friendly test utility - abstract getProperty(el: Element, name: string): any; abstract dispatchEvent(el: any, evt: any): any; - - // Used by router - abstract log(error: any): any; - abstract logGroup(error: any): any; - abstract logGroupEnd(): any; + abstract readonly supportsDOMEvents: boolean; // Used by Meta - abstract remove(el: any): Node; + abstract remove(el: any): void; abstract createElement(tagName: any, doc?: any): HTMLElement; abstract createHtmlDocument(): HTMLDocument; abstract getDefaultDocument(): Document; @@ -53,25 +48,17 @@ export abstract class DomAdapter { // Used by KeyEventsPlugin abstract onAndCancel(el: any, evt: any, listener: any): Function; - abstract supportsDOMEvents(): boolean; // Used by PlatformLocation and ServerEventManagerPlugin abstract getGlobalEventTarget(doc: Document, target: string): any; // Used by PlatformLocation - abstract getHistory(): History; - abstract getLocation(): - any; /** This is the ambient Location definition, NOT Location from @angular/common. */ abstract getBaseHref(doc: Document): string|null; abstract resetBaseElement(): void; // TODO: remove dependency in DefaultValueAccessor abstract getUserAgent(): string; - // Used by AngularProfiler - abstract performanceNow(): number; - - // Used by CookieXSRFStrategy - abstract supportsCookies(): boolean; + // Used in the legacy @angular/http package which has some usage in g3. abstract getCookie(name: string): string|null; } diff --git a/packages/common/src/location/platform_location.ts b/packages/common/src/location/platform_location.ts index 06c1b81cf9..42371b9e5e 100644 --- a/packages/common/src/location/platform_location.ts +++ b/packages/common/src/location/platform_location.ts @@ -120,8 +120,8 @@ export class BrowserPlatformLocation extends PlatformLocation { // This is moved to its own method so that `MockPlatformLocationStrategy` can overwrite it /** @internal */ _init() { - (this as {location: Location}).location = getDOM().getLocation(); - this._history = getDOM().getHistory(); + (this as {location: Location}).location = window.location; + this._history = window.history; } getBaseHrefFromDOM(): string { diff --git a/packages/core/test/application_ref_spec.ts b/packages/core/test/application_ref_spec.ts index 2f97e91745..b3b6fa09ed 100644 --- a/packages/core/test/application_ref_spec.ts +++ b/packages/core/test/application_ref_spec.ts @@ -59,7 +59,7 @@ class SomeComponent { const errorHandler = new ErrorHandler(); (errorHandler as any)._console = mockConsole as any; - const platformModule = getDOM().supportsDOMEvents() ? + const platformModule = getDOM().supportsDOMEvents ? BrowserModule : require('@angular/platform-server').ServerModule; diff --git a/packages/core/test/dom/dom_adapter_spec.ts b/packages/core/test/dom/dom_adapter_spec.ts index ba1dab5bc1..859b12a716 100644 --- a/packages/core/test/dom/dom_adapter_spec.ts +++ b/packages/core/test/dom/dom_adapter_spec.ts @@ -14,7 +14,7 @@ import {isTextNode} from '@angular/platform-browser/testing/src/browser_util'; describe('dom adapter', () => { let defaultDoc: any; beforeEach(() => { - defaultDoc = getDOM().supportsDOMEvents() ? document : getDOM().createHtmlDocument(); + defaultDoc = getDOM().supportsDOMEvents ? document : getDOM().createHtmlDocument(); }); it('should be able to create text nodes and use them with the other APIs', () => { @@ -36,7 +36,7 @@ import {isTextNode} from '@angular/platform-browser/testing/src/browser_util'; expect(() => getDOM().remove(d)).not.toThrow(); }); - if (getDOM().supportsDOMEvents()) { + 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 index 69e969a79a..11ee3e15a3 100644 --- a/packages/core/test/linker/integration_spec.ts +++ b/packages/core/test/linker/integration_spec.ts @@ -109,8 +109,7 @@ function declareTests(config?: {useJit: boolean}) { fixture.componentInstance.ctxProp = 'Hello World!'; fixture.detectChanges(); - expect(getDOM().getProperty(fixture.debugElement.children[0].nativeElement, 'id')) - .toEqual('Hello World!'); + expect(fixture.debugElement.children[0].nativeElement.id).toEqual('Hello World!'); }); it('should consume binding to aria-* attributes', () => { @@ -168,13 +167,11 @@ function declareTests(config?: {useJit: boolean}) { const fixture = TestBed.createComponent(MyComp); fixture.detectChanges(); - expect(getDOM().getProperty(fixture.debugElement.children[0].nativeElement, 'tabIndex')) - .toEqual(0); + expect(fixture.debugElement.children[0].nativeElement.tabIndex).toEqual(0); fixture.componentInstance.ctxNumProp = 5; fixture.detectChanges(); - expect(getDOM().getProperty(fixture.debugElement.children[0].nativeElement, 'tabIndex')) - .toEqual(5); + expect(fixture.debugElement.children[0].nativeElement.tabIndex).toEqual(5); }); it('should consume binding to camel-cased properties', () => { @@ -184,13 +181,11 @@ function declareTests(config?: {useJit: boolean}) { const fixture = TestBed.createComponent(MyComp); fixture.detectChanges(); - expect(getDOM().getProperty(fixture.debugElement.children[0].nativeElement, 'readOnly')) - .toBeFalsy(); + expect(fixture.debugElement.children[0].nativeElement.readOnly).toBeFalsy(); fixture.componentInstance.ctxBoolProp = true; fixture.detectChanges(); - expect(getDOM().getProperty(fixture.debugElement.children[0].nativeElement, 'readOnly')) - .toBeTruthy(); + expect(fixture.debugElement.children[0].nativeElement.readOnly).toBeTruthy(); }); it('should consume binding to innerHtml', () => { @@ -236,7 +231,7 @@ function declareTests(config?: {useJit: boolean}) { fixture.debugElement.componentInstance.ctxProp = 'foo'; fixture.detectChanges(); - expect(getDOM().getProperty(nativeEl, 'htmlFor')).toBe('foo'); + expect(nativeEl.htmlFor).toBe('foo'); }); it('should consume directive watch expression change.', () => { @@ -617,7 +612,7 @@ function declareTests(config?: {useJit: boolean}) { expect(cmp.numberOfChecks).toEqual(2); }); - if (getDOM().supportsDOMEvents()) { + if (getDOM().supportsDOMEvents) { it('should allow to destroy a component from within a host event handler', fakeAsync(() => { TestBed.configureTestingModule({declarations: [MyComp, [[PushCmpWithHostEvent]]]}); @@ -922,7 +917,7 @@ function declareTests(config?: {useJit: boolean}) { fixture.detectChanges(); - expect(getDOM().getProperty(tc.nativeElement, 'id')).toEqual('newId'); + expect(tc.nativeElement.id).toEqual('newId'); }); it('should not use template variables for expressions in hostProperties', () => { @@ -996,7 +991,7 @@ function declareTests(config?: {useJit: boolean}) { - if (getDOM().supportsDOMEvents()) { + if (getDOM().supportsDOMEvents) { it('should support preventing default on render events', () => { TestBed.configureTestingModule({ declarations: @@ -1881,7 +1876,7 @@ function declareTests(config?: {useJit: boolean}) { fixture.detectChanges(); const el = fixture.nativeElement.querySelector('span'); - expect(getDOM().getProperty(el, 'title')).toEqual('TITLE'); + expect(el.title).toEqual('TITLE'); }); }); @@ -2097,7 +2092,7 @@ function declareTests(config?: {useJit: boolean}) { expect(fixture.debugElement.children[0].nativeElement.outerHTML).toContain('my-attr="aaa"'); }); - if (getDOM().supportsDOMEvents()) { + if (getDOM().supportsDOMEvents) { it('should support event decorators', fakeAsync(() => { TestBed.configureTestingModule({ declarations: [MyComp, DirectiveWithPropDecorators], @@ -2200,7 +2195,7 @@ function declareTests(config?: {useJit: boolean}) { })); }); - if (getDOM().supportsDOMEvents()) { + if (getDOM().supportsDOMEvents) { describe('svg', () => { it('should support svg elements', () => { TestBed.configureTestingModule({declarations: [MyComp]}); @@ -2211,12 +2206,10 @@ function declareTests(config?: {useJit: boolean}) { const el = fixture.nativeElement; const svg = el.childNodes[0]; const use = svg.childNodes[0]; - expect(getDOM().getProperty(svg, 'namespaceURI')) - .toEqual('http://www.w3.org/2000/svg'); - expect(getDOM().getProperty(use, 'namespaceURI')) - .toEqual('http://www.w3.org/2000/svg'); + expect(svg.namespaceURI).toEqual('http://www.w3.org/2000/svg'); + expect(use.namespaceURI).toEqual('http://www.w3.org/2000/svg'); - const firstAttribute = getDOM().getProperty(use, 'attributes')[0]; + const firstAttribute = use.attributes[0]; expect(firstAttribute.name).toEqual('xlink:href'); expect(firstAttribute.namespaceURI).toEqual('http://www.w3.org/1999/xlink'); }); @@ -2232,12 +2225,9 @@ function declareTests(config?: {useJit: boolean}) { const svg = el.childNodes[0]; const foreignObject = svg.childNodes[0]; const p = foreignObject.childNodes[0]; - expect(getDOM().getProperty(svg, 'namespaceURI')) - .toEqual('http://www.w3.org/2000/svg'); - expect(getDOM().getProperty(foreignObject, 'namespaceURI')) - .toEqual('http://www.w3.org/2000/svg'); - expect(getDOM().getProperty(p, 'namespaceURI')) - .toEqual('http://www.w3.org/1999/xhtml'); + expect(svg.namespaceURI).toEqual('http://www.w3.org/2000/svg'); + expect(foreignObject.namespaceURI).toEqual('http://www.w3.org/2000/svg'); + expect(p.namespaceURI).toEqual('http://www.w3.org/1999/xhtml'); }); }); diff --git a/packages/core/test/linker/projection_integration_spec.ts b/packages/core/test/linker/projection_integration_spec.ts index d97673918f..b6f43389c8 100644 --- a/packages/core/test/linker/projection_integration_spec.ts +++ b/packages/core/test/linker/projection_integration_spec.ts @@ -508,7 +508,7 @@ describe('projection', () => { }); } - if (getDOM().supportsDOMEvents()) { + if (getDOM().supportsDOMEvents) { it('should support non emulated styles', () => { TestBed.configureTestingModule({declarations: [OtherComp]}); TestBed.overrideComponent(MainComp, { diff --git a/packages/core/test/linker/query_list_spec.ts b/packages/core/test/linker/query_list_spec.ts index f79b137548..c5b1fb7070 100644 --- a/packages/core/test/linker/query_list_spec.ts +++ b/packages/core/test/linker/query_list_spec.ts @@ -156,7 +156,7 @@ import {beforeEach, describe, expect, it} from '@angular/core/testing/src/testin expect(data.length).toBe(0); }); - if (getDOM().supportsDOMEvents()) { + if (getDOM().supportsDOMEvents) { describe('simple observable interface', () => { it('should fire callbacks on change', fakeAsync(() => { let fires = 0; diff --git a/packages/core/test/linker/regression_integration_spec.ts b/packages/core/test/linker/regression_integration_spec.ts index d4a1956368..b92e1a04ca 100644 --- a/packages/core/test/linker/regression_integration_spec.ts +++ b/packages/core/test/linker/regression_integration_spec.ts @@ -462,7 +462,7 @@ function declareTestsUsingBootstrap() { destroyPlatform(); }); - if (getDOM().supportsDOMEvents()) { + if (getDOM().supportsDOMEvents) { // This test needs a real DOM.... it('should keep change detecting if there was an error', (done) => { diff --git a/packages/core/test/linker/security_integration_spec.ts b/packages/core/test/linker/security_integration_spec.ts index 65fb81296a..006be4f715 100644 --- a/packages/core/test/linker/security_integration_spec.ts +++ b/packages/core/test/linker/security_integration_spec.ts @@ -49,13 +49,9 @@ function declareTests(config?: {useJit: boolean}) { }); }); - let originalLog: (msg: any) => any; beforeEach(() => { - originalLog = getDOM().log; - getDOM().log = (msg) => { /* disable logging */ }; - }); - afterEach(() => { - getDOM().log = originalLog; + // Disable logging for these tests. + spyOn(console, 'log').and.callFake(() => {}); }); describe('events', () => { @@ -128,7 +124,7 @@ function declareTests(config?: {useJit: boolean}) { cmp.detectChanges(); const div = cmp.debugElement.children[0]; expect(div.injector.get(OnPrefixDir).onclick).toBe(value); - expect(getDOM().getProperty(div.nativeElement, 'onclick')).not.toBe(value); + expect(div.nativeElement.onclick).not.toBe(value); expect(div.nativeElement.hasAttribute('onclick')).toEqual(false); }); }); @@ -145,7 +141,7 @@ function declareTests(config?: {useJit: boolean}) { const trusted = sanitizer.bypassSecurityTrustUrl('javascript:alert(1)'); ci.ctxProp = trusted; fixture.detectChanges(); - expect(getDOM().getProperty(e, 'href')).toEqual('javascript:alert(1)'); + expect(e.getAttribute('href')).toEqual('javascript:alert(1)'); }); it('should error when using the wrong trusted value', () => { @@ -171,25 +167,21 @@ function declareTests(config?: {useJit: boolean}) { const ci = fixture.componentInstance; ci.ctxProp = trusted; fixture.detectChanges(); - expect(getDOM().getProperty(e, 'href')).toMatch(/SafeValue(%20| )must(%20| )use/); + expect(e.href).toMatch(/SafeValue(%20| )must(%20| )use/); }); }); describe('sanitizing', () => { - function checkEscapeOfHrefProperty(fixture: ComponentFixture, isAttribute: boolean) { + function checkEscapeOfHrefProperty(fixture: ComponentFixture) { const e = fixture.debugElement.children[0].nativeElement; const ci = fixture.componentInstance; ci.ctxProp = 'hello'; fixture.detectChanges(); - // In the browser, reading href returns an absolute URL. On the server side, - // it just echoes back the property. - let value = isAttribute ? e.getAttribute('href') : getDOM().getProperty(e, 'href'); - expect(value).toMatch(/.*\/?hello$/); + expect(e.getAttribute('href')).toMatch(/.*\/?hello$/); ci.ctxProp = 'javascript:alert(1)'; fixture.detectChanges(); - value = isAttribute ? e.getAttribute('href') : getDOM().getProperty(e, 'href'); - expect(value).toEqual('unsafe:javascript:alert(1)'); + expect(e.getAttribute('href')).toEqual('unsafe:javascript:alert(1)'); } it('should escape unsafe properties', () => { @@ -197,7 +189,7 @@ function declareTests(config?: {useJit: boolean}) { TestBed.overrideComponent(SecuredComponent, {set: {template}}); const fixture = TestBed.createComponent(SecuredComponent); - checkEscapeOfHrefProperty(fixture, false); + checkEscapeOfHrefProperty(fixture); }); it('should escape unsafe attributes', () => { @@ -205,7 +197,7 @@ function declareTests(config?: {useJit: boolean}) { TestBed.overrideComponent(SecuredComponent, {set: {template}}); const fixture = TestBed.createComponent(SecuredComponent); - checkEscapeOfHrefProperty(fixture, true); + checkEscapeOfHrefProperty(fixture); }); it('should escape unsafe properties if they are used in host bindings', () => { @@ -220,7 +212,7 @@ function declareTests(config?: {useJit: boolean}) { TestBed.overrideComponent(SecuredComponent, {set: {template}}); const fixture = TestBed.createComponent(SecuredComponent); - checkEscapeOfHrefProperty(fixture, false); + checkEscapeOfHrefProperty(fixture); }); it('should escape unsafe attributes if they are used in host bindings', () => { @@ -235,7 +227,7 @@ function declareTests(config?: {useJit: boolean}) { TestBed.overrideComponent(SecuredComponent, {set: {template}}); const fixture = TestBed.createComponent(SecuredComponent); - checkEscapeOfHrefProperty(fixture, true); + checkEscapeOfHrefProperty(fixture); }); 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 index d2228dbb69..9977909ff4 100644 --- a/packages/core/test/view/element_spec.ts +++ b/packages/core/test/view/element_spec.ts @@ -87,8 +87,8 @@ const removeEventListener = 'removeEventListener'; Services.checkAndUpdateView(view); const el = rootNodes[0]; - expect(getDOM().getProperty(el, 'title')).toBe('v1'); - expect(getDOM().getProperty(el, 'value')).toBe('v2'); + expect(el.title).toBe('v1'); + expect(el.value).toBe('v2'); }); }); }); diff --git a/packages/core/test/view/helper.ts b/packages/core/test/view/helper.ts index d362f6646f..52cd8ae419 100644 --- a/packages/core/test/view/helper.ts +++ b/packages/core/test/view/helper.ts @@ -12,7 +12,7 @@ import {ArgumentType, initServicesIfNeeded, NodeCheckFn, NodeDef, rootRenderNode import {TestBed} from '@angular/core/testing'; export function isBrowser() { - return getDOM().supportsDOMEvents(); + return getDOM().supportsDOMEvents; } export const ARG_TYPE_VALUES = [ArgumentType.Inline, ArgumentType.Dynamic]; diff --git a/packages/platform-browser/src/browser/browser_adapter.ts b/packages/platform-browser/src/browser/browser_adapter.ts index fcdc656551..fb0b79ee93 100644 --- a/packages/platform-browser/src/browser/browser_adapter.ts +++ b/packages/platform-browser/src/browser/browser_adapter.ts @@ -7,20 +7,9 @@ */ import {ɵparseCookieValue as parseCookieValue, ɵsetRootDomAdapter as setRootDomAdapter} from '@angular/common'; -import {ɵglobal as global} from '@angular/core'; import {GenericBrowserDomAdapter} from './generic_browser_adapter'; -const nodeContains: (this: Node, other: Node) => boolean = (() => { - if (global['Node']) { - return global['Node'].prototype.contains || function(this: Node, node: any) { - return !!(this.compareDocumentPosition(node) & 16); - }; - } - - return undefined as any; -})(); - /** * A `DomAdapter` powered by full browser DOM APIs. * @@ -32,27 +21,6 @@ export class BrowserDomAdapter extends GenericBrowserDomAdapter { static makeCurrent() { setRootDomAdapter(new BrowserDomAdapter()); } - getProperty(el: Node, name: string): any { - return (el)[name]; - } - - log(error: string): void { - if (window.console) { - window.console.log && window.console.log(error); - } - } - - logGroup(error: string): void { - if (window.console) { - window.console.group && window.console.group(error); - } - } - - logGroupEnd(): void { - if (window.console) { - window.console.groupEnd && window.console.groupEnd(); - } - } onAndCancel(el: Node, evt: any, listener: any): Function { el.addEventListener(evt, listener, false); @@ -65,14 +33,10 @@ export class BrowserDomAdapter extends GenericBrowserDomAdapter { dispatchEvent(el: Node, evt: any) { el.dispatchEvent(evt); } - remove(node: Node): Node { + remove(node: Node): void { if (node.parentNode) { node.parentNode.removeChild(node); } - return node; - } - getValue(el: any): string { - return el.value; } createElement(tagName: string, doc?: Document): HTMLElement { doc = doc || this.getDefaultDocument(); @@ -105,12 +69,6 @@ export class BrowserDomAdapter extends GenericBrowserDomAdapter { } return null; } - getHistory(): History { - return window.history; - } - getLocation(): Location { - return window.location; - } getBaseHref(doc: Document): string|null { const href = getBaseElementHref(); return href == null ? null : relativePath(href); @@ -121,17 +79,6 @@ export class BrowserDomAdapter extends GenericBrowserDomAdapter { getUserAgent(): string { return window.navigator.userAgent; } - performanceNow(): number { - // performance.now() is not available in all browsers, see - // https://caniuse.com/high-resolution-time - return window.performance && window.performance.now ? window.performance.now() : - new Date().getTime(); - } - - supportsCookies(): boolean { - return true; - } - getCookie(name: string): string|null { return parseCookieValue(document.cookie, name); } @@ -139,22 +86,15 @@ export class BrowserDomAdapter extends GenericBrowserDomAdapter { let baseElement: HTMLElement|null = null; function getBaseElementHref(): string|null { - if (!baseElement) { - baseElement = document.querySelector('base')!; - if (!baseElement) { - return null; - } - } - return baseElement.getAttribute('href'); + baseElement = baseElement || document.querySelector('base'); + return baseElement ? baseElement.getAttribute('href') : null; } // based on urlUtils.js in AngularJS 1 -let urlParsingNode: any; +let urlParsingNode: HTMLAnchorElement|undefined; function relativePath(url: any): string { - if (!urlParsingNode) { - urlParsingNode = document.createElement('a'); - } + urlParsingNode = urlParsingNode || document.createElement('a'); urlParsingNode.setAttribute('href', url); - return (urlParsingNode.pathname.charAt(0) === '/') ? urlParsingNode.pathname : - '/' + urlParsingNode.pathname; + const pathName = urlParsingNode.pathname; + return pathName.charAt(0) === '/' ? pathName : `/${pathName}`; } diff --git a/packages/platform-browser/src/browser/generic_browser_adapter.ts b/packages/platform-browser/src/browser/generic_browser_adapter.ts index fd42d1d746..c74f9f53c5 100644 --- a/packages/platform-browser/src/browser/generic_browser_adapter.ts +++ b/packages/platform-browser/src/browser/generic_browser_adapter.ts @@ -17,11 +17,5 @@ import {ɵDomAdapter as DomAdapter} from '@angular/common'; * can introduce XSS risks. */ export abstract class GenericBrowserDomAdapter extends DomAdapter { - constructor() { - super(); - } - - supportsDOMEvents(): boolean { - return true; - } + readonly supportsDOMEvents: boolean = true; } diff --git a/packages/platform-browser/src/browser/tools/common_tools.ts b/packages/platform-browser/src/browser/tools/common_tools.ts index 3136bcbfde..4a51cfae32 100644 --- a/packages/platform-browser/src/browser/tools/common_tools.ts +++ b/packages/platform-browser/src/browser/tools/common_tools.ts @@ -6,7 +6,6 @@ * found in the LICENSE file at https://angular.io/license */ -import {ɵgetDOM as getDOM} from '@angular/common'; import {ApplicationRef, ComponentRef} from '@angular/core'; import {window} from './browser'; @@ -50,13 +49,13 @@ export class AngularProfiler { if (record && isProfilerAvailable) { window.console.profile(profileName); } - const start = getDOM().performanceNow(); + const start = performanceNow(); let numTicks = 0; - while (numTicks < 5 || (getDOM().performanceNow() - start) < 500) { + while (numTicks < 5 || (performanceNow() - start) < 500) { this.appRef.tick(); numTicks++; } - const end = getDOM().performanceNow(); + const end = performanceNow(); if (record && isProfilerAvailable) { window.console.profileEnd(profileName); } @@ -67,3 +66,8 @@ export class AngularProfiler { return new ChangeDetectionPerfRecord(msPerTick, numTicks); } } + +function performanceNow() { + return window.performance && window.performance.now ? window.performance.now() : + new Date().getTime(); +} diff --git a/packages/platform-browser/test/browser/bootstrap_spec.ts b/packages/platform-browser/test/browser/bootstrap_spec.ts index 6a9eacb5f6..d49260773f 100644 --- a/packages/platform-browser/test/browser/bootstrap_spec.ts +++ b/packages/platform-browser/test/browser/bootstrap_spec.ts @@ -273,7 +273,7 @@ function bootstrap( }); })); - if (getDOM().supportsDOMEvents()) { + if (getDOM().supportsDOMEvents) { it('should forward the error to promise when bootstrap fails', inject([AsyncTestCompleter], (async: AsyncTestCompleter) => { const logger = new MockConsole(); diff --git a/packages/platform-browser/test/dom/events/event_manager_spec.ts b/packages/platform-browser/test/dom/events/event_manager_spec.ts index 0369cd615c..0f08c19018 100644 --- a/packages/platform-browser/test/dom/events/event_manager_spec.ts +++ b/packages/platform-browser/test/dom/events/event_manager_spec.ts @@ -21,7 +21,7 @@ let zone: NgZone; describe('EventManager', () => { beforeEach(() => { - doc = getDOM().supportsDOMEvents() ? document : getDOM().createHtmlDocument(); + doc = getDOM().supportsDOMEvents ? document : getDOM().createHtmlDocument(); zone = new NgZone({}); domEventPlugin = new DomEventsPlugin(doc); }); @@ -337,7 +337,7 @@ describe('EventManager', () => { it('should only trigger one Change detection when bubbling with shouldCoalesceEventChangeDetection = true', (done: DoneFn) => { - doc = getDOM().supportsDOMEvents() ? document : getDOM().createHtmlDocument(); + doc = getDOM().supportsDOMEvents ? document : getDOM().createHtmlDocument(); zone = new NgZone({shouldCoalesceEventChangeDetection: true}); domEventPlugin = new DomEventsPlugin(doc); const element = el('
'); @@ -374,7 +374,7 @@ describe('EventManager', () => { it('should only trigger one Change detection when bubbling with shouldCoalesceRunChangeDetection = true', (done: DoneFn) => { - doc = getDOM().supportsDOMEvents() ? document : getDOM().createHtmlDocument(); + doc = getDOM().supportsDOMEvents ? document : getDOM().createHtmlDocument(); zone = new NgZone({shouldCoalesceRunChangeDetection: true}); domEventPlugin = new DomEventsPlugin(doc); const element = el('
'); @@ -411,7 +411,7 @@ describe('EventManager', () => { it('should not drain micro tasks queue too early with shouldCoalesceEventChangeDetection=true', (done: DoneFn) => { - doc = getDOM().supportsDOMEvents() ? document : getDOM().createHtmlDocument(); + doc = getDOM().supportsDOMEvents ? document : getDOM().createHtmlDocument(); zone = new NgZone({shouldCoalesceEventChangeDetection: true}); domEventPlugin = new DomEventsPlugin(doc); const element = el('
'); @@ -457,7 +457,7 @@ describe('EventManager', () => { it('should not drain micro tasks queue too early with shouldCoalesceRunChangeDetection=true', (done: DoneFn) => { - doc = getDOM().supportsDOMEvents() ? document : getDOM().createHtmlDocument(); + doc = getDOM().supportsDOMEvents ? document : getDOM().createHtmlDocument(); zone = new NgZone({shouldCoalesceRunChangeDetection: true}); domEventPlugin = new DomEventsPlugin(doc); const element = el('
'); diff --git a/packages/platform-server/src/domino_adapter.ts b/packages/platform-server/src/domino_adapter.ts index 54256a2c19..39d1c8f81d 100644 --- a/packages/platform-server/src/domino_adapter.ts +++ b/packages/platform-server/src/domino_adapter.ts @@ -10,10 +10,6 @@ const domino = require('domino'); import {ɵBrowserDomAdapter as BrowserDomAdapter} from '@angular/platform-browser'; import {ɵsetRootDomAdapter as setRootDomAdapter} from '@angular/common'; -function _notImplemented(methodName: string) { - return new Error('This method is not implemented in DominoAdapter: ' + methodName); -} - export function setDomTypes() { // Make all Domino types available in the global env. Object.assign(global, domino.impl); @@ -45,23 +41,9 @@ export class DominoAdapter extends BrowserDomAdapter { setRootDomAdapter(new DominoAdapter()); } + readonly supportsDOMEvents = false; private static defaultDoc: Document; - log(error: string) { - // tslint:disable-next-line:no-console - console.log(error); - } - - logGroup(error: string) { - console.error(error); - } - - logGroupEnd() {} - - supportsDOMEvents(): boolean { - return false; - } - createHtmlDocument(): HTMLDocument { return parseDocument('fakeTitle'); } @@ -80,18 +62,6 @@ export class DominoAdapter extends BrowserDomAdapter { return node.shadowRoot == node; } - getProperty(el: Element, name: string): any { - if (name === 'href') { - // Domino tries to resolve href-s which we do not want. Just return the - // attribute value. - return el.getAttribute('href'); - } else if (name === 'innerText') { - // Domino does not support innerText. Just map it to textContent. - return el.textContent; - } - return (el)[name]; - } - getGlobalEventTarget(doc: Document, target: string): EventTarget|null { if (target === 'window') { return doc.defaultView; @@ -106,13 +76,8 @@ export class DominoAdapter extends BrowserDomAdapter { } getBaseHref(doc: Document): string { - const base = doc.documentElement!.querySelector('base'); - let href = ''; - if (base) { - href = base.getAttribute('href')!; - } // TODO(alxhub): Need relative path logic from BrowserDomAdapter here? - return href; + return doc.documentElement!.querySelector('base')?.getAttribute('href') || ''; } dispatchEvent(el: Node, evt: any) { @@ -126,24 +91,11 @@ export class DominoAdapter extends BrowserDomAdapter { } } - getHistory(): History { - throw _notImplemented('getHistory'); - } - getLocation(): Location { - throw _notImplemented('getLocation'); - } getUserAgent(): string { return 'Fake user agent'; } - performanceNow(): number { - return Date.now(); - } - - supportsCookies(): boolean { - return false; - } getCookie(name: string): string { - throw _notImplemented('getCookie'); + throw new Error('getCookie has not been implemented'); } } diff --git a/packages/platform-server/test/integration_spec.ts b/packages/platform-server/test/integration_spec.ts index 11245bf6cb..3f781f98a7 100644 --- a/packages/platform-server/test/integration_spec.ts +++ b/packages/platform-server/test/integration_spec.ts @@ -402,7 +402,7 @@ class HiddenModule { } (function() { -if (getDOM().supportsDOMEvents()) return; // NODE only +if (getDOM().supportsDOMEvents) return; // NODE only describe('platform-server integration', () => { beforeEach(() => { diff --git a/packages/platform-server/test/server_styles_host_spec.ts b/packages/platform-server/test/server_styles_host_spec.ts index 5a0056fb13..75e3454f8a 100644 --- a/packages/platform-server/test/server_styles_host_spec.ts +++ b/packages/platform-server/test/server_styles_host_spec.ts @@ -11,7 +11,7 @@ import {ServerStylesHost} from '@angular/platform-server/src/styles_host'; (function() { -if (getDOM().supportsDOMEvents()) return; // NODE only +if (getDOM().supportsDOMEvents) return; // NODE only describe('ServerStylesHost', () => { let ssh: ServerStylesHost; diff --git a/packages/router/src/router_module.ts b/packages/router/src/router_module.ts index 810fd68a11..51293c5fd2 100644 --- a/packages/router/src/router_module.ts +++ b/packages/router/src/router_module.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {APP_BASE_HREF, HashLocationStrategy, Location, LOCATION_INITIALIZED, LocationStrategy, PathLocationStrategy, PlatformLocation, ViewportScroller, ɵgetDOM as getDOM} from '@angular/common'; +import {APP_BASE_HREF, HashLocationStrategy, Location, LOCATION_INITIALIZED, LocationStrategy, PathLocationStrategy, PlatformLocation, ViewportScroller} from '@angular/common'; import {ANALYZE_FOR_ENTRY_COMPONENTS, APP_BOOTSTRAP_LISTENER, APP_INITIALIZER, ApplicationRef, Compiler, ComponentRef, Inject, Injectable, InjectionToken, Injector, ModuleWithProviders, NgModule, NgModuleFactoryLoader, NgProbeToken, Optional, Provider, SkipSelf, SystemJsNgModuleLoader} from '@angular/core'; import {of, Subject} from 'rxjs'; @@ -448,12 +448,13 @@ export function setupRouter( assignExtraOptionsToRouter(opts, router); if (opts.enableTracing) { - const dom = getDOM(); router.events.subscribe((e: Event) => { - dom.logGroup(`Router Event: ${(e.constructor).name}`); - dom.log(e.toString()); - dom.log(e); - dom.logGroupEnd(); + // tslint:disable:no-console + console.group?.(`Router Event: ${(e.constructor).name}`); + console.log(e.toString()); + console.log(e); + console.groupEnd?.(); + // tslint:enable:no-console }); }