From 39b324de6774d151ddffe0e0122cd93e01a57d62 Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Tue, 4 Mar 2025 16:46:50 +0100 Subject: [PATCH] refactor(core): drop render-related perf counters (#60200) It looks like we are not getting enough value from having those counters. This is an exploratory PR to see if thsoe could be removed. PR Close #60200 --- .../core/src/linker/view_container_ref.ts | 1 - .../core/src/render3/dom_node_manipulation.ts | 10 --- packages/core/src/render3/i18n/i18n_apply.ts | 3 - .../core/src/render3/instructions/listener.ts | 1 - .../core/src/render3/instructions/shared.ts | 3 - .../core/src/render3/node_manipulation.ts | 6 -- packages/core/src/render3/util/attrs_utils.ts | 2 - packages/core/src/util/ng_dev_mode.ts | 38 -------- .../core/test/acceptance/listener_spec.ts | 17 +--- .../test/acceptance/renderer_factory_spec.ts | 4 - packages/core/test/acceptance/styling_spec.ts | 50 ----------- .../acceptance/view_container_ref_spec.ts | 4 - .../test/render3/instructions/styling_spec.ts | 45 ---------- .../core/test/render3/instructions_spec.ts | 86 +------------------ 14 files changed, 3 insertions(+), 267 deletions(-) diff --git a/packages/core/src/linker/view_container_ref.ts b/packages/core/src/linker/view_container_ref.ts index bec830a9723..9de0e2e9e70 100644 --- a/packages/core/src/linker/view_container_ref.ts +++ b/packages/core/src/linker/view_container_ref.ts @@ -738,7 +738,6 @@ export function createContainerRef( */ function insertAnchorNode(hostLView: LView, hostTNode: TNode): RComment { const renderer = hostLView[RENDERER]; - ngDevMode && ngDevMode.rendererCreateComment++; const commentNode = renderer.createComment(ngDevMode ? 'container' : ''); const hostNative = getNativeByTNode(hostTNode, hostLView)!; diff --git a/packages/core/src/render3/dom_node_manipulation.ts b/packages/core/src/render3/dom_node_manipulation.ts index c6c87946eb2..7b3ac069418 100644 --- a/packages/core/src/render3/dom_node_manipulation.ts +++ b/packages/core/src/render3/dom_node_manipulation.ts @@ -14,18 +14,14 @@ import {setUpAttributes} from './util/attrs_utils'; import {TNode} from './interfaces/node'; export function createTextNode(renderer: Renderer, value: string): RText { - ngDevMode && ngDevMode.rendererCreateTextNode++; - ngDevMode && ngDevMode.rendererSetText++; return renderer.createText(value); } export function updateTextNode(renderer: Renderer, rNode: RText, value: string): void { - ngDevMode && ngDevMode.rendererSetText++; renderer.setValue(rNode, value); } export function createCommentNode(renderer: Renderer, value: string): RComment { - ngDevMode && ngDevMode.rendererCreateComment++; return renderer.createComment(escapeCommentText(value)); } @@ -41,7 +37,6 @@ export function createElementNode( name: string, namespace: string | null, ): RElement { - ngDevMode && ngDevMode.rendererCreateElement++; return renderer.createElement(name, namespace); } @@ -56,12 +51,10 @@ export function nativeInsertBefore( beforeNode: RNode | null, isMove: boolean, ): void { - ngDevMode && ngDevMode.rendererInsertBefore++; renderer.insertBefore(parent, child, beforeNode, isMove); } export function nativeAppendChild(renderer: Renderer, parent: RElement, child: RNode): void { - ngDevMode && ngDevMode.rendererAppendChild++; ngDevMode && assertDefined(parent, 'parent node must be defined'); renderer.appendChild(parent, child); } @@ -90,7 +83,6 @@ export function nativeAppendOrInsertBefore( * @param isHostElement A flag indicating if a node to be removed is a host of a component. */ export function nativeRemoveNode(renderer: Renderer, rNode: RNode, isHostElement?: boolean): void { - ngDevMode && ngDevMode.rendererRemoveNode++; renderer.removeChild(null, rNode, isHostElement); } @@ -116,7 +108,6 @@ export function clearElementContents(rElement: RElement): void { function writeDirectStyle(renderer: Renderer, element: RElement, newValue: string) { ngDevMode && assertString(newValue, "'newValue' should be a string"); renderer.setAttribute(element, 'style', newValue); - ngDevMode && ngDevMode.rendererSetStyle++; } /** @@ -137,7 +128,6 @@ function writeDirectClass(renderer: Renderer, element: RElement, newValue: strin } else { renderer.setAttribute(element, 'class', newValue); } - ngDevMode && ngDevMode.rendererSetClassName++; } /** Sets up the static DOM attributes on an `RNode`. */ diff --git a/packages/core/src/render3/i18n/i18n_apply.ts b/packages/core/src/render3/i18n/i18n_apply.ts index 09245a95705..01c2f785e64 100644 --- a/packages/core/src/render3/i18n/i18n_apply.ts +++ b/packages/core/src/render3/i18n/i18n_apply.ts @@ -258,7 +258,6 @@ export function applyMutableOpCodes( if (typeof opCode == 'string') { const textNodeIndex = mutableOpCodes[++i] as number; if (lView[textNodeIndex] === null) { - ngDevMode && ngDevMode.rendererCreateTextNode++; ngDevMode && assertIndexInRange(lView, textNodeIndex); lView[textNodeIndex] = _locateOrCreateNode(lView, textNodeIndex, opCode, Node.TEXT_NODE); } @@ -344,7 +343,6 @@ export function applyMutableOpCodes( 'string', `Expected "${commentValue}" to be a comment node value`, ); - ngDevMode && ngDevMode.rendererCreateComment++; ngDevMode && assertIndexInExpandoRange(lView, commentNodeIndex); const commentRNode = (lView[commentNodeIndex] = _locateOrCreateNode( lView, @@ -367,7 +365,6 @@ export function applyMutableOpCodes( `Expected "${tagName}" to be an element node tag name`, ); - ngDevMode && ngDevMode.rendererCreateElement++; ngDevMode && assertIndexInExpandoRange(lView, elementNodeIndex); const elementRNode = (lView[elementNodeIndex] = _locateOrCreateNode( lView, diff --git a/packages/core/src/render3/instructions/listener.ts b/packages/core/src/render3/instructions/listener.ts index a05770fc7e0..cde73841ee4 100644 --- a/packages/core/src/render3/instructions/listener.ts +++ b/packages/core/src/render3/instructions/listener.ts @@ -220,7 +220,6 @@ export function listenerInternal( listenerFn = wrapListener(tNode, lView, context, listenerFn); stashEventListener(target as RElement, eventName, listenerFn); const cleanupFn = renderer.listen(target as RElement, eventName, listenerFn); - ngDevMode && ngDevMode.rendererAddEventListener++; lCleanup.push(listenerFn, cleanupFn); tCleanup && tCleanup.push(eventName, idxOrTargetGetter, lCleanupIndex, lCleanupIndex + 1); diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index 7f8cf886947..65fcb9a7439 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -269,7 +269,6 @@ export function elementPropertyInternal( if (!isPropertyValid(element, propName, tNode.value, tView.schemas)) { handleUnknownPropertyError(propName, tNode.value, tNode.type, lView); } - ngDevMode.rendererSetProperty++; } // It is assumed that the sanitizer is only added when the compiler determines that the @@ -493,10 +492,8 @@ export function setElementAttribute( sanitizer: SanitizerFn | null | undefined, ) { if (value == null) { - ngDevMode && ngDevMode.rendererRemoveAttribute++; renderer.removeAttribute(element, name, namespace); } else { - ngDevMode && ngDevMode.rendererSetAttribute++; const strValue = sanitizer == null ? renderStringify(value) : sanitizer(value, tagName || '', name); diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index 637adef065e..3764b1d751d 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -138,7 +138,6 @@ function applyToElementOrContainer( } else if (action === WalkTNodeTreeAction.Detach) { nativeRemoveNode(renderer, rNode, isComponent); } else if (action === WalkTNodeTreeAction.Destroy) { - ngDevMode && ngDevMode.rendererDestroyNode++; renderer.destroyNode!(rNode); } if (lContainer != null) { @@ -321,7 +320,6 @@ function cleanUpView(tView: TView, lView: LView): void { processCleanups(tView, lView); // For component views only, the local renderer is destroyed at clean up time. if (lView[TVIEW].type === TViewType.Component) { - ngDevMode && ngDevMode.rendererDestroy++; lView[RENDERER].destroy(); } @@ -977,16 +975,13 @@ export function applyStyling( if (isClassBased) { // We actually want JS true/false here because any truthy value should add the class if (!value) { - ngDevMode && ngDevMode.rendererRemoveClass++; renderer.removeClass(rNode, prop); } else { - ngDevMode && ngDevMode.rendererAddClass++; renderer.addClass(rNode, prop); } } else { let flags = prop.indexOf('-') === -1 ? undefined : (RendererStyleFlags2.DashCase as number); if (value == null /** || value === undefined */) { - ngDevMode && ngDevMode.rendererRemoveStyle++; renderer.removeStyle(rNode, prop, flags); } else { // A value is important if it ends with `!important`. The style @@ -999,7 +994,6 @@ export function applyStyling( flags! |= RendererStyleFlags2.Important; } - ngDevMode && ngDevMode.rendererSetStyle++; renderer.setStyle(rNode, prop, value, flags); } } diff --git a/packages/core/src/render3/util/attrs_utils.ts b/packages/core/src/render3/util/attrs_utils.ts index f3736626e00..5f0a41404f8 100644 --- a/packages/core/src/render3/util/attrs_utils.ts +++ b/packages/core/src/render3/util/attrs_utils.ts @@ -57,14 +57,12 @@ export function setUpAttributes(renderer: Renderer, native: RElement, attrs: TAt const namespaceURI = attrs[i++] as string; const attrName = attrs[i++] as string; const attrVal = attrs[i++] as string; - ngDevMode && ngDevMode.rendererSetAttribute++; renderer.setAttribute(native, attrName, attrVal, namespaceURI); } else { // attrName is string; const attrName = value as string; const attrVal = attrs[++i]; // Standard attributes - ngDevMode && ngDevMode.rendererSetAttribute++; if (isAnimationProp(attrName)) { renderer.setProperty(native, attrName, attrVal); } else { diff --git a/packages/core/src/util/ng_dev_mode.ts b/packages/core/src/util/ng_dev_mode.ts index f98f88b9aef..4aee0cf9d0b 100644 --- a/packages/core/src/util/ng_dev_mode.ts +++ b/packages/core/src/util/ng_dev_mode.ts @@ -30,25 +30,6 @@ declare global { firstCreatePass: number; tNode: number; tView: number; - rendererCreateTextNode: number; - rendererSetText: number; - rendererCreateElement: number; - rendererAddEventListener: number; - rendererSetAttribute: number; - rendererRemoveAttribute: number; - rendererSetProperty: number; - rendererSetClassName: number; - rendererAddClass: number; - rendererRemoveClass: number; - rendererSetStyle: number; - rendererRemoveStyle: number; - rendererDestroy: number; - rendererDestroyNode: number; - rendererMoveNode: number; - rendererRemoveNode: number; - rendererAppendChild: number; - rendererInsertBefore: number; - rendererCreateComment: number; hydratedNodes: number; hydratedComponents: number; dehydratedViewsRemoved: number; @@ -65,25 +46,6 @@ export function ngDevModeResetPerfCounters(): NgDevModePerfCounters { firstCreatePass: 0, tNode: 0, tView: 0, - rendererCreateTextNode: 0, - rendererSetText: 0, - rendererCreateElement: 0, - rendererAddEventListener: 0, - rendererSetAttribute: 0, - rendererRemoveAttribute: 0, - rendererSetProperty: 0, - rendererSetClassName: 0, - rendererAddClass: 0, - rendererRemoveClass: 0, - rendererSetStyle: 0, - rendererRemoveStyle: 0, - rendererDestroy: 0, - rendererDestroyNode: 0, - rendererMoveNode: 0, - rendererRemoveNode: 0, - rendererAppendChild: 0, - rendererInsertBefore: 0, - rendererCreateComment: 0, hydratedNodes: 0, hydratedComponents: 0, dehydratedViewsRemoved: 0, diff --git a/packages/core/test/acceptance/listener_spec.ts b/packages/core/test/acceptance/listener_spec.ts index d5d2e72bac4..c23ad9794a0 100644 --- a/packages/core/test/acceptance/listener_spec.ts +++ b/packages/core/test/acceptance/listener_spec.ts @@ -25,10 +25,6 @@ import { import {TestBed} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; -function getNoOfNativeListeners(): number { - return ngDevMode ? ngDevMode.rendererAddEventListener : 0; -} - describe('event listeners', () => { describe('even handling statements', () => { it('should call function on event emit', () => { @@ -371,17 +367,12 @@ describe('event listeners', () => { TestBed.configureTestingModule({ declarations: [TestCmpt, WithClicksCmpt, LikesClicks, MdButton], }); - const noOfEventListenersRegisteredSoFar = getNoOfNativeListeners(); + const fixture = TestBed.createComponent(TestCmpt); fixture.detectChanges(); const buttonDebugEls = fixture.debugElement.queryAll(By.css('button')); const withClicksEls = fixture.debugElement.queryAll(By.css('with-clicks-cmpt')); - // We want to assert that only one native event handler was registered but still all - // directives are notified when an event fires. This assertion can only be verified in - // the ngDevMode (but the coalescing always happens!). - ngDevMode && expect(getNoOfNativeListeners()).toBe(noOfEventListenersRegisteredSoFar + 2); - buttonDebugEls[0].nativeElement.click(); expect(withClicksEls[0].injector.get(WithClicksCmpt).counter).toBe(1); expect(buttonDebugEls[0].injector.get(LikesClicks).counter).toBe(1); @@ -412,16 +403,10 @@ describe('event listeners', () => { } TestBed.configureTestingModule({declarations: [TestCmpt, LikesClicks]}); - const noOfEventListenersRegisteredSoFar = getNoOfNativeListeners(); const fixture = TestBed.createComponent(TestCmpt); fixture.detectChanges(); const buttonDebugEl = fixture.debugElement.query(By.css('button')); - // We want to assert that only one native event handler was registered but still all - // directives are notified when an event fires. This assertion can only be verified in - // the ngDevMode (but the coalescing always happens!). - ngDevMode && expect(getNoOfNativeListeners()).toBe(noOfEventListenersRegisteredSoFar + 1); - buttonDebugEl.nativeElement.click(); expect(buttonDebugEl.injector.get(LikesClicks).counter).toBe(1); expect(fixture.componentInstance.counter).toBe(1); diff --git a/packages/core/test/acceptance/renderer_factory_spec.ts b/packages/core/test/acceptance/renderer_factory_spec.ts index 8d2bf0161dd..a63fa0342ae 100644 --- a/packages/core/test/acceptance/renderer_factory_spec.ts +++ b/packages/core/test/acceptance/renderer_factory_spec.ts @@ -503,8 +503,6 @@ describe('Renderer2 destruction hooks', () => { fixture.detectChanges(); expect(fixture.nativeElement.textContent).toBe(''); - expect(ngDevMode!.rendererDestroy).toBe(0); - expect(ngDevMode!.rendererDestroyNode).toBe(3); }); it('should call renderer.destroy for each component destroyed', () => { @@ -517,8 +515,6 @@ describe('Renderer2 destruction hooks', () => { fixture.detectChanges(); expect(fixture.nativeElement.textContent).toBe(''); - expect(ngDevMode!.rendererDestroy).toBe(3); - expect(ngDevMode!.rendererDestroyNode).toBe(3); }); }); diff --git a/packages/core/test/acceptance/styling_spec.ts b/packages/core/test/acceptance/styling_spec.ts index 6555e6f958d..499f4b39a18 100644 --- a/packages/core/test/acceptance/styling_spec.ts +++ b/packages/core/test/acceptance/styling_spec.ts @@ -923,11 +923,6 @@ describe('styling', () => { const div = fixture.nativeElement.querySelector('div') as HTMLDivElement; expect(div.style.backgroundImage).toBe('url("#test")'); - - expectPerfCounters({ - rendererSetStyle: 1, - tNode: 2, - }); }); it('should set !important on a single property', () => { @@ -948,11 +943,6 @@ describe('styling', () => { // We have to check the `style` attribute, because `element.style.prop` doesn't include // `!important`. Use a regex, because the different renderers produce different whitespace. expect(html).toMatch(/style=["|']width:\s*50px\s*!important/); - - expectPerfCounters({ - rendererSetStyle: 1, - tNode: 2, - }); }); it('should set !important that is not preceded by a space', () => { @@ -973,11 +963,6 @@ describe('styling', () => { // We have to check the `style` attribute, because `element.style.prop` doesn't include // `!important`. Use a regex, because the different renderers produce different whitespace. expect(html).toMatch(/style=["|']width:\s*50px\s*!important/); - - expectPerfCounters({ - rendererSetStyle: 1, - tNode: 2, - }); }); it('should set !important on a dash-case property', () => { @@ -998,11 +983,6 @@ describe('styling', () => { // We have to check the `style` attribute, because `element.style.prop` doesn't include // `!important`. Use a regex, because the different renderers produce different whitespace. expect(html).toMatch(/style=["|']margin-right:\s*5px\s*!important/); - - expectPerfCounters({ - rendererSetStyle: 1, - tNode: 2, - }); }); it('should set !important on multiple properties', () => { @@ -1023,11 +1003,6 @@ describe('styling', () => { // We have to check the `style` attribute, because `element.style.prop` doesn't include // `!important`. Use a regex, because the different renderers produce different whitespace. expect(html).toMatch(/style=["|']height:\s*25px\s*!important;\s*width:\s*50px\s*!important/); - - expectPerfCounters({ - rendererSetStyle: 2, - tNode: 2, - }); }); it('should set !important if some properties are !important and other are not', () => { @@ -1048,11 +1023,6 @@ describe('styling', () => { // We have to check the `style` attribute, because `element.style.prop` doesn't include // `!important`. Use a regex, because the different renderers produce different whitespace. expect(html).toMatch(/style=["|']height:\s*25px;\s*width:\s*50px\s*!important/); - - expectPerfCounters({ - rendererSetStyle: 2, - tNode: 2, - }); }); it('should not write to the native element if a directive shadows the class input', () => { @@ -1948,7 +1918,6 @@ describe('styling', () => { expect(element.style.fontSize).toEqual('100px'); // once for the template flush and again for the host bindings - expect(ngDevMode!.rendererSetStyle).toEqual(4); ngDevModeResetPerfCounters(); component.opacity = '0.6'; @@ -1961,9 +1930,6 @@ describe('styling', () => { expect(element.style.width).toEqual('100px'); expect(element.style.height).toEqual('100px'); expect(element.style.fontSize).toEqual('50px'); - - // once for the template flush and again for the host bindings - expect(ngDevMode!.rendererSetStyle).toEqual(4); }); it('should combine all styling across the template, directive and component host bindings', () => { @@ -2255,7 +2221,6 @@ describe('styling', () => { fixture.detectChanges(); const element = fixture.nativeElement.querySelector('div'); - assertStyleCounters(4, 0); assertStyle(element, 'width', '111px'); assertStyle(element, 'height', '111px'); @@ -2263,7 +2228,6 @@ describe('styling', () => { ngDevModeResetPerfCounters(); fixture.detectChanges(); - assertStyleCounters(1, 0); assertStyle(element, 'width', '222px'); assertStyle(element, 'height', '111px'); @@ -2271,7 +2235,6 @@ describe('styling', () => { ngDevModeResetPerfCounters(); fixture.detectChanges(); - assertStyleCounters(1, 0); assertStyle(element, 'width', '222px'); assertStyle(element, 'height', '222px'); @@ -2279,7 +2242,6 @@ describe('styling', () => { ngDevModeResetPerfCounters(); fixture.detectChanges(); - assertStyleCounters(1, 0); assertStyle(element, 'width', '555px'); assertStyle(element, 'height', '222px'); @@ -2296,7 +2258,6 @@ describe('styling', () => { fixture.detectChanges(); // No change, hence no write - assertStyleCounters(0, 0); assertStyle(element, 'width', '123px'); assertStyle(element, 'height', '123px'); @@ -2304,7 +2265,6 @@ describe('styling', () => { ngDevModeResetPerfCounters(); fixture.detectChanges(); - assertStyleCounters(1, 0); assertStyle(element, 'width', '999px'); assertStyle(element, 'height', '123px'); @@ -2313,7 +2273,6 @@ describe('styling', () => { fixture.detectChanges(); // the width is only applied once - assertStyleCounters(1, 0); assertStyle(element, 'width', '0px'); assertStyle(element, 'height', '123px'); @@ -2321,7 +2280,6 @@ describe('styling', () => { ngDevModeResetPerfCounters(); fixture.detectChanges(); - assertStyleCounters(2, 0); assertStyle(element, 'width', '1000px'); assertStyle(element, 'height', '123px'); assertStyle(element, 'color', 'red'); @@ -2332,7 +2290,6 @@ describe('styling', () => { // height gets applied twice and all other // values get applied - assertStyleCounters(1, 0); assertStyle(element, 'width', '1000px'); assertStyle(element, 'height', '1100px'); assertStyle(element, 'color', 'red'); @@ -2341,7 +2298,6 @@ describe('styling', () => { ngDevModeResetPerfCounters(); fixture.detectChanges(); - assertStyleCounters(3, 0); assertStyle(element, 'width', '2000px'); assertStyle(element, 'height', '1100px'); assertStyle(element, 'color', 'blue'); @@ -2352,7 +2308,6 @@ describe('styling', () => { fixture.detectChanges(); // all four are applied because the map was altered - assertStyleCounters(0, 1); assertStyle(element, 'width', '2000px'); assertStyle(element, 'height', '1100px'); assertStyle(element, 'color', 'blue'); @@ -4144,11 +4099,6 @@ describe('styling', () => { }); }); -function assertStyleCounters(countForSet: number, countForRemove: number) { - expect(ngDevMode!.rendererSetStyle).toEqual(countForSet); - expect(ngDevMode!.rendererRemoveStyle).toEqual(countForRemove); -} - function assertStyle(element: HTMLElement, prop: string, value: any) { expect((element.style as any)[prop]).toEqual(value); } diff --git a/packages/core/test/acceptance/view_container_ref_spec.ts b/packages/core/test/acceptance/view_container_ref_spec.ts index 00e0bcf1c5c..04f054c7c64 100644 --- a/packages/core/test/acceptance/view_container_ref_spec.ts +++ b/packages/core/test/acceptance/view_container_ref_spec.ts @@ -913,7 +913,6 @@ describe('ViewContainerRef', () => { expect(() => vcRefDir.vcref.detach(-1)).toThrow(); expect(() => vcRefDir.vcref.detach(42)).toThrow(); - expect(ngDevMode!.rendererDestroyNode).toBe(0); }); it('should detach the last embedded view when no index is specified', () => { @@ -935,7 +934,6 @@ describe('ViewContainerRef', () => { fixture.detectChanges(); expect(getElementHtml(fixture.nativeElement)).toEqual('

ABCD'); expect(viewE.destroyed).toBeFalsy(); - expect(ngDevMode!.rendererDestroyNode).toBe(0); }); it('should not throw when destroying a detached component view', () => { @@ -1013,7 +1011,6 @@ describe('ViewContainerRef', () => { expect(() => vcRefDir.vcref.remove(-1)).toThrow(); expect(() => vcRefDir.vcref.remove(42)).toThrow(); - expect(ngDevMode!.rendererDestroyNode).toBe(2); }); it('should remove the last embedded view when no index is specified', () => { @@ -1035,7 +1032,6 @@ describe('ViewContainerRef', () => { fixture.detectChanges(); expect(getElementHtml(fixture.nativeElement)).toEqual('

ABCD'); expect(viewE.destroyed).toBeTruthy(); - expect(ngDevMode!.rendererDestroyNode).toBe(1); }); it('should throw when trying to insert a removed or destroyed view', () => { diff --git a/packages/core/test/render3/instructions/styling_spec.ts b/packages/core/test/render3/instructions/styling_spec.ts index 084eb09ebea..d82e4df3cc3 100644 --- a/packages/core/test/render3/instructions/styling_spec.ts +++ b/packages/core/test/render3/instructions/styling_spec.ts @@ -33,7 +33,6 @@ import {HEADER_OFFSET, TVIEW} from '@angular/core/src/render3/interfaces/view'; import {getLView, leaveView, setBindingRootForHostBindings} from '@angular/core/src/render3/state'; import {getNativeByIndex} from '@angular/core/src/render3/util/view_utils'; import {keyValueArraySet} from '@angular/core/src/util/array_utils'; -import {ngDevModeResetPerfCounters} from '@angular/core/src/util/ng_dev_mode'; import {getElementClasses, getElementStyles} from '@angular/core/testing/src/styling'; import {clearFirstUpdatePass, enterViewWithOneDiv, rewindBindingIndex} from './shared_spec'; @@ -79,58 +78,40 @@ describe('styling', () => { }); it('should set style based on priority', () => { - ngDevModeResetPerfCounters(); ɵɵstyleProp('color', 'red'); ɵɵstyleProp('color', 'blue'); // Higher priority, should win. expectStyle(div).toEqual({color: 'blue'}); - // The intermediate value has to set since it does not know if it is last one. - expect(ngDevMode!.rendererSetStyle).toEqual(2); - ngDevModeResetPerfCounters(); clearFirstUpdatePass(); rewindBindingIndex(); ɵɵstyleProp('color', 'red'); // no change ɵɵstyleProp('color', 'green'); // change to green expectStyle(div).toEqual({color: 'green'}); - expect(ngDevMode!.rendererSetStyle).toEqual(1); - ngDevModeResetPerfCounters(); rewindBindingIndex(); ɵɵstyleProp('color', 'black'); // First binding update expectStyle(div).toEqual({color: 'green'}); // Green still has priority. - expect(ngDevMode!.rendererSetStyle).toEqual(0); - ngDevModeResetPerfCounters(); rewindBindingIndex(); ɵɵstyleProp('color', 'black'); // no change ɵɵstyleProp('color', undefined); // Clear second binding expectStyle(div).toEqual({color: 'black'}); // default to first binding - expect(ngDevMode!.rendererSetStyle).toEqual(1); - ngDevModeResetPerfCounters(); rewindBindingIndex(); ɵɵstyleProp('color', null); expectStyle(div).toEqual({}); // default to first binding - expect(ngDevMode!.rendererSetStyle).toEqual(0); - expect(ngDevMode!.rendererRemoveStyle).toEqual(1); }); it('should set class based on priority', () => { - ngDevModeResetPerfCounters(); ɵɵclassProp('foo', false); ɵɵclassProp('foo', true); // Higher priority, should win. expectClass(div).toEqual({foo: true}); - expect(ngDevMode!.rendererAddClass).toEqual(1); - ngDevModeResetPerfCounters(); clearFirstUpdatePass(); rewindBindingIndex(); ɵɵclassProp('foo', false); // no change ɵɵclassProp('foo', undefined); // change (have no opinion) expectClass(div).toEqual({}); - expect(ngDevMode!.rendererAddClass).toEqual(0); - expect(ngDevMode!.rendererRemoveClass).toEqual(1); - ngDevModeResetPerfCounters(); rewindBindingIndex(); ɵɵclassProp('foo', false); // no change @@ -145,67 +126,41 @@ describe('styling', () => { describe('styleMap', () => { it('should work with maps', () => { - ngDevModeResetPerfCounters(); ɵɵstyleMap({}); expectStyle(div).toEqual({}); - expect(ngDevMode!.rendererSetStyle).toEqual(0); - expect(ngDevMode!.rendererRemoveStyle).toEqual(0); - ngDevModeResetPerfCounters(); clearFirstUpdatePass(); rewindBindingIndex(); ɵɵstyleMap({color: 'blue'}); expectStyle(div).toEqual({color: 'blue'}); - expect(ngDevMode!.rendererSetStyle).toEqual(1); - expect(ngDevMode!.rendererRemoveStyle).toEqual(0); - ngDevModeResetPerfCounters(); rewindBindingIndex(); ɵɵstyleMap({color: 'red'}); expectStyle(div).toEqual({color: 'red'}); - expect(ngDevMode!.rendererSetStyle).toEqual(1); - expect(ngDevMode!.rendererRemoveStyle).toEqual(0); - ngDevModeResetPerfCounters(); rewindBindingIndex(); ɵɵstyleMap({color: null, width: '100px'}); expectStyle(div).toEqual({width: '100px'}); - expect(ngDevMode!.rendererSetStyle).toEqual(1); - expect(ngDevMode!.rendererRemoveStyle).toEqual(1); - ngDevModeResetPerfCounters(); }); it('should work with object literal and strings', () => { - ngDevModeResetPerfCounters(); ɵɵstyleMap(''); expectStyle(div).toEqual({}); - expect(ngDevMode!.rendererSetStyle).toEqual(0); - expect(ngDevMode!.rendererRemoveStyle).toEqual(0); - ngDevModeResetPerfCounters(); clearFirstUpdatePass(); rewindBindingIndex(); ɵɵstyleMap('color: blue'); expectStyle(div).toEqual({color: 'blue'}); - expect(ngDevMode!.rendererSetStyle).toEqual(1); - expect(ngDevMode!.rendererRemoveStyle).toEqual(0); - ngDevModeResetPerfCounters(); rewindBindingIndex(); ɵɵstyleMap('color: red'); expectStyle(div).toEqual({color: 'red'}); - expect(ngDevMode!.rendererSetStyle).toEqual(1); - expect(ngDevMode!.rendererRemoveStyle).toEqual(0); - ngDevModeResetPerfCounters(); rewindBindingIndex(); ɵɵstyleMap('width: 100px'); expectStyle(div).toEqual({width: '100px'}); - expect(ngDevMode!.rendererSetStyle).toEqual(1); - expect(ngDevMode!.rendererRemoveStyle).toEqual(1); - ngDevModeResetPerfCounters(); }); it('should collaborate with properties', () => { diff --git a/packages/core/test/render3/instructions_spec.ts b/packages/core/test/render3/instructions_spec.ts index dd69b582ef7..667dcd59645 100644 --- a/packages/core/test/render3/instructions_spec.ts +++ b/packages/core/test/render3/instructions_spec.ts @@ -6,10 +6,7 @@ * found in the LICENSE file at https://angular.dev/license */ -import {CommonModule} from '@angular/common'; import {Component, Input} from '@angular/core/public_api'; -import {ngDevModeResetPerfCounters} from '@angular/core/src/util/ng_dev_mode'; -import {TestBed} from '@angular/core/testing'; import {getSortedClassName} from '@angular/core/testing/src/styling'; import { @@ -77,8 +74,7 @@ describe('instructions', () => { }); describe('bind', () => { - it('should update bindings when value changes with the correct perf counters', () => { - ngDevModeResetPerfCounters(); + it('should update bindings when value changes', () => { const t = new ViewFixture({create: createAnchor, decls: 1, vars: 1}); t.update(() => { @@ -90,19 +86,9 @@ describe('instructions', () => { ɵɵproperty('title', 'World'); }); expect(t.html).toEqual(''); - expect(ngDevMode).toEqual( - jasmine.objectContaining({ - firstCreatePass: 1, - tNode: 2, // 1 for hostElement + 1 for the template under test - tView: 2, // 1 for rootView + 1 for the template view - rendererCreateElement: 1, - rendererSetProperty: 2, - }), - ); }); it('should not update bindings when value does not change, with the correct perf counters', () => { - ngDevModeResetPerfCounters(); const idempotentUpdate = () => { ɵɵproperty('title', 'Hello'); }; @@ -118,21 +104,11 @@ describe('instructions', () => { t.update(); expect(t.html).toEqual(''); - expect(ngDevMode).toEqual( - jasmine.objectContaining({ - firstCreatePass: 1, - tNode: 2, // 1 for hostElement + 1 for the template under test - tView: 2, // 1 for rootView + 1 for the template view - rendererCreateElement: 1, - rendererSetProperty: 1, - }), - ); }); }); describe('element', () => { - it('should create an element with the correct perf counters', () => { - ngDevModeResetPerfCounters(); + it('should create an element', () => { const t = new ViewFixture({ create: () => { ɵɵelement(0, 'div', 0); @@ -145,14 +121,6 @@ describe('instructions', () => { const div = (t.host as HTMLElement).querySelector('div')!; expect(div.id).toEqual('test'); expect(div.title).toEqual('Hello'); - expect(ngDevMode).toEqual( - jasmine.objectContaining({ - firstCreatePass: 1, - tNode: 2, // 1 for div, 1 for host element - tView: 2, // 1 for rootView + 1 for the template view - rendererCreateElement: 1, - }), - ); }); it('should instantiate nodes at high indices', () => { @@ -193,7 +161,6 @@ describe('instructions', () => { describe('attribute', () => { it('should use sanitizer function', () => { - ngDevModeResetPerfCounters(); const t = new ViewFixture({create: createDiv, decls: 1, vars: 1}); t.update(() => { @@ -205,15 +172,6 @@ describe('instructions', () => { ɵɵattribute('title', bypassSanitizationTrustUrl('javascript:true'), ɵɵsanitizeUrl); }); expect(t.html).toEqual('
'); - expect(ngDevMode).toEqual( - jasmine.objectContaining({ - firstCreatePass: 1, - tNode: 2, // 1 for div, 1 for host element - tView: 2, // 1 for rootView + 1 for the template view - rendererCreateElement: 1, - rendererSetAttribute: 2, - }), - ); }); }); @@ -224,7 +182,6 @@ describe('instructions', () => { * is not producing chained instructions yet. */ it('should chain', () => { - ngDevModeResetPerfCounters(); //
const t = new ViewFixture({create: createDiv, update: () => {}, decls: 1, vars: 2}); t.update(() => { @@ -235,15 +192,6 @@ describe('instructions', () => { ɵɵproperty('title', 'two')('accessKey', 'B'); }); expect(t.html).toEqual('
'); - expect(ngDevMode).toEqual( - jasmine.objectContaining({ - firstCreatePass: 1, - tNode: 2, // 1 for div, 1 for host element - tView: 2, // 1 for rootView + 1 for the template view - rendererCreateElement: 1, - rendererSetProperty: 4, - }), - ); }); }); @@ -318,36 +266,6 @@ describe('instructions', () => { }); }); - describe('performance counters', () => { - it('should create tView only once for each nested level', () => { - @Component({ - selector: 'nested-loops', - template: ` - - `, - imports: [CommonModule], - }) - class NestedLoops { - rows = [ - ['a', 'b'], - ['A', 'B'], - ['a', 'b'], - ['A', 'B'], - ]; - } - ngDevModeResetPerfCounters(); - TestBed.createComponent(NestedLoops); - expect(ngDevMode).toEqual( - jasmine.objectContaining({ - // Expect: component view + ngFor(row) + ngFor(col) - tView: 3, // should be: 3 - }), - ); - }); - }); - describe('sanitization injection compatibility', () => { it('should work for url sanitization', () => { const s = new LocalMockSanitizer((value) => `${value}-sanitized`);