Skip to content

perf(forms): avoid scheduling change detection on blur when already touched #68636

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
arturovt wants to merge 1 commit into
base: main
Choose a base branch
from
+42 −1
Open

perf(forms): avoid scheduling change detection on blur when already touched #68636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion packages/forms/signals/src/directive/control_native.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,16 @@ export function nativeControlCreate(
};
// Pass undefined as the raw value since the parse function doesn't care about it.
host.listenToDom('input', () => parser.setRawValue(undefined));
host.listenToDom('blur', () => parent.state().markAsTouched());

// Use addEventListener directly so the blur handler does not unconditionally call
// markViewDirty. When the control is already touched, markAsTouched() is a no-op signal
// write and no CD should be scheduled; when it isn't, the signal write itself schedules CD.
// In a large application, every avoided change detection cycle is a win: fewer things
// Angular has to check, less time spent traversing the component tree.
const nativeEl = host.nativeElement;
const blurListener = () => parent.state().markAsTouched();
nativeEl.addEventListener('blur', blurListener);
parent.destroyRef.onDestroy(() => nativeEl.removeEventListener('blur', blurListener));

// TODO: move extraction to first update pass?
if (isInput(input) && inputRequiresValidityTracking(input)) {
Expand Down
32 changes: 32 additions & 0 deletions packages/forms/signals/test/web/form_field.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
viewChildren,
ViewContainerRef,
ViewEncapsulation,
ɵChangeDetectionScheduler,
} from '@angular/core';
import {TestBed} from '@angular/core/testing';

Expand Down Expand Up @@ -5096,6 +5097,37 @@ describe('field directive', () => {
expect(field().touched()).toBe(true);
});

it('should not notify the change detection scheduler on blur when control is already touched', () => {
@Component({
imports: [FormField],
template: `<input [formField]="f" />`,
})
class TestCmp {
f = form(signal(''));
}

const fixture = act(() => TestBed.createComponent(TestCmp));
const inputEl = fixture.nativeElement.firstChild as HTMLInputElement;
const field = fixture.componentInstance.f;

// Put the control into the already-touched state so the next blur is a no-op.
act(() => field().markAsTouched());
expect(field().touched()).toBe(true);

const scheduler = TestBed.inject(ɵChangeDetectionScheduler);
const notifySpy = spyOn(scheduler, 'notify');

// Blur when already touched — nothing changed, so CD must not be scheduled.
//
// Regression: when the blur listener was registered via host.listenToDom('blur', ...),
// wrapListener called markViewDirty unconditionally before running the callback, which
// called scheduler.notify(NotificationSource.Listener) even when selfTouched was
// already true and the signal write was a no-op.
inputEl.dispatchEvent(new Event('blur'));

expect(notifySpy).not.toHaveBeenCalled();
});

it('should synchronize with custom control touched status', () => {
@Component({
selector: 'my-input',
Expand Down
Loading