Visitar URL original
perf(forms): use addEventListener to avoid unconditional CD schedulin… · angular/angular@cc515b8 · GitHub
Skip to content

Commit cc515b8

Browse files
arturovtamishne
arturovt
authored and
amishne
committed
perf(forms): use addEventListener to avoid unconditional CD scheduling on blur
Previously, the blur and input listeners in nativeControlCreate were registered via host.listenToDom(), which routes through wrapListener and calls markViewDirty before running the callback. This caused scheduler.notify() to fire on every blur event, even when the control was already touched and the signal write was a no-op. Switch to native addEventListener so that change detection ticks are only triggered by actual signal changes. Remove the now-unused listenToDom method from ControlDirectiveHost and its implementation. (cherry picked from commit 6d36d6c)
1 parent 5761e25 commit cc515b8

5 files changed

Lines changed: 53 additions & 28 deletions

File tree

‎packages/core/src/render3/instructions/control.ts‎

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import type {DirectiveDef} from '../interfaces/definition';
1414
import {InputFlags} from '../interfaces/input_flags';
1515
import {type TNode, TNodeFlags} from '../interfaces/node';
1616
import {isComponentHost} from '../interfaces/type_checks';
17-
import {type LView, RENDERER, type TView} from '../interfaces/view';
17+
import type {LView, TView} from '../interfaces/view';
1818
import {
1919
getCurrentTNode,
2020
getLView,
@@ -25,7 +25,7 @@ import {
2525
import {getNativeByTNode} from '../util/view_utils';
2626
import {debugStringifyTypeForError} from '../util/stringify_utils';
2727
import {listenToDirectiveOutput} from '../view/directive_outputs';
28-
import {listenToDomEvent, wrapListener} from '../view/listeners';
28+
import {wrapListener} from '../view/listeners';
2929
import {setDirectiveInput} from './shared';
3030
import {writeToDirectiveInput} from './write_to_directive_input';
3131

@@ -145,19 +145,6 @@ class ControlDirectiveHostImpl implements ControlDirectiveHost {
145145
);
146146
}
147147

148-
listenToDom(eventName: string, listener: (event: Event) => void): void {
149-
listenToDomEvent(
150-
this.tNode,
151-
this.tView,
152-
this.lView as LView<{} | null>,
153-
undefined,
154-
this.lView[RENDERER],
155-
eventName,
156-
listener,
157-
wrapListener(this.tNode, this.lView, listener),
158-
);
159-
}
160-
161148
setInputOnDirectives(
162149
inputName: string,
163150
value: unknown,

‎packages/core/src/render3/interfaces/control.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,4 @@ export interface ControlDirectiveHost<_TPassthroughInput extends string | undefi
8888
value: unknown,
8989
writePredicate?: (currentValue: unknown) => boolean,
9090
): boolean;
91-
92-
/**
93-
* Listens to a DOM event on the host element.
94-
*/
95-
listenToDom(eventName: string, listener: (event: Event) => void): void;
9691
}

‎packages/core/test/bundling/create_component/bundle.golden_symbols.json‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,6 @@
238238
"_platformInjector",
239239
"_processI18nInsertBefore",
240240
"_retrieveHydrationInfoImpl",
241-
"_stashEventListenerImpl",
242241
"_wasLastNodeCreated",
243242
"activeConsumer",
244243
"addAfterRenderSequencesForView",
@@ -376,7 +375,6 @@
376375
"extractDefListOrFactory",
377376
"extractDirectiveDef",
378377
"finalizeConsumerAfterComputation",
379-
"findExistingListener",
380378
"findMatchingDehydratedView",
381379
"forEachSingleProvider",
382380
"forkInnerZoneWithAngularBehavior",
@@ -509,7 +507,6 @@
509507
"invokeDirectivesHostBindings",
510508
"invokeHostBindingsInCreationMode",
511509
"isAngularZoneProperty",
512-
"isAnimationEventType",
513510
"isAnimationProp",
514511
"isApplicationBootstrapConfig",
515512
"isComponentDef",
@@ -518,7 +515,6 @@
518515
"isCurrentTNodeParent",
519516
"isDestroyed",
520517
"isDetachedByI18n",
521-
"isDirectiveHost",
522518
"isEnvironmentProviders",
523519
"isErrorLike",
524520
"isExistingProvider",
@@ -553,7 +549,6 @@
553549
"leavingNodes",
554550
"linkTNodeInTView",
555551
"listenToDirectiveOutput",
556-
"listenToDomEvent",
557552
"listenToOutput",
558553
"locateDirectiveOrProvider",
559554
"locateHostElement",
@@ -684,7 +679,6 @@
684679
"shouldSearchParent",
685680
"signalSetFn",
686681
"signalValueChanged",
687-
"stashEventListenerImpl",
688682
"storeLViewOnDestroy",
689683
"storeListenerCleanup",
690684
"stringifyCSSSelector",

‎packages/forms/signals/src/directive/control_native.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,20 @@ export function nativeControlCreate(
6060
bindings['controlValue'] = value;
6161
setNativeControlValue(input, value);
6262
};
63+
64+
// Use `addEventListener` so we only trigger ticks on signal changes.
65+
// See https://github.com/angular/angular/issues/61839.
66+
const nativeEl = host.nativeElement;
6367
// Pass undefined as the raw value since the parse function doesn't care about it.
64-
host.listenToDom('input', () => parser.setRawValue(undefined));
65-
host.listenToDom('blur', () => parent.state().markAsTouched());
68+
const onInput = () => parser.setRawValue(undefined);
69+
nativeEl.addEventListener('input', onInput);
70+
const onBlur = () => parent.state().markAsTouched();
71+
nativeEl.addEventListener('blur', onBlur);
72+
73+
parent.destroyRef.onDestroy(() => {
74+
nativeEl.removeEventListener('input', onInput);
75+
nativeEl.removeEventListener('blur', onBlur);
76+
});
6677

6778
// TODO: move extraction to first update pass?
6879
if (isInput(input) && inputRequiresValidityTracking(input)) {

‎packages/forms/signals/test/web/form_field.spec.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,14 @@ import {
2525
numberAttribute,
2626
output,
2727
Output,
28+
provideZonelessChangeDetection,
2829
resource,
2930
signal,
3031
viewChild,
3132
viewChildren,
3233
ViewContainerRef,
3334
ViewEncapsulation,
35+
ɵChangeDetectionScheduler,
3436
} from '@angular/core';
3537
import {TestBed} from '@angular/core/testing';
3638

@@ -5185,6 +5187,42 @@ describe('field directive', () => {
51855187
expect(field().touched()).toBe(true);
51865188
});
51875189

5190+
it('should not notify the change detection scheduler on blur when control is already touched', () => {
5191+
// provideZonelessChangeDetection is required so that ChangeDetectionScheduler is
5192+
// wired into the LView environment. In zone mode the scheduler slot is null and
5193+
// markViewDirty never calls notify(), making the test meaningless without it.
5194+
TestBed.configureTestingModule({providers: [provideZonelessChangeDetection()]});
5195+
5196+
@Component({
5197+
imports: [FormField],
5198+
template: `<input [formField]="f" />`,
5199+
})
5200+
class TestCmp {
5201+
f = form(signal(''));
5202+
}
5203+
5204+
const fixture = act(() => TestBed.createComponent(TestCmp));
5205+
const inputEl = fixture.nativeElement.firstChild as HTMLInputElement;
5206+
const field = fixture.componentInstance.f;
5207+
5208+
// Put the control into the already-touched state so the next blur is a no-op.
5209+
act(() => field().markAsTouched());
5210+
expect(field().touched()).toBe(true);
5211+
5212+
const scheduler = TestBed.inject(ɵChangeDetectionScheduler);
5213+
const notifySpy = spyOn(scheduler, 'notify');
5214+
5215+
// Blur when already touched — nothing changed, so CD must not be scheduled.
5216+
//
5217+
// Regression: when the blur listener was registered via host.listenToDom('blur', ...),
5218+
// wrapListener called markViewDirty unconditionally before running the callback, which
5219+
// called scheduler.notify(NotificationSource.Listener) even when selfTouched was
5220+
// already true and the signal write was a no-op.
5221+
inputEl.dispatchEvent(new Event('blur'));
5222+
5223+
expect(notifySpy).not.toHaveBeenCalled();
5224+
});
5225+
51885226
it('should synchronize with custom control touched status', () => {
51895227
@Component({
51905228
selector: 'my-input',

0 commit comments

Comments
 (0)