Skip to content

Commit 41be02d

Browse files
leonsenftatscott
authored andcommitted
perf(forms): implement change detection for field control bindings
For each field state property, check if it has changed since the last time it was checked before writing it the corresponding form control property. The `pattern` and `required` properties of the field state now return a default value rather than `undefined` if not defined by metadata.
1 parent 5b210e9 commit 41be02d

6 files changed

Lines changed: 523 additions & 129 deletions

File tree

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/control_bindings/control_bindings.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ MyComponent.ɵcmp = /* @__PURE__ */i0.ɵɵdefineComponent({
22
type: MyComponent,
33
selectors: [["ng-component"]],
44
decls: 4,
5-
vars: 2,
5+
vars: 3,
66
consts: [["field", "Not a form control"], [3, "field"]],
77
template: function MyComponent_Template(rf, ctx) {
88
if (rf & 1) {

packages/compiler/src/template/pipeline/src/phases/var_counting.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,6 @@ function varsUsedByOp(op: (ir.CreateOp | ir.UpdateOp) & ir.ConsumesVarsTrait): n
119119
return slots;
120120
case ir.OpKind.Property:
121121
case ir.OpKind.DomProperty:
122-
case ir.OpKind.Control:
123122
slots = 1;
124123

125124
// We need to assign a slot even for singleton interpolations, because the
@@ -128,6 +127,10 @@ function varsUsedByOp(op: (ir.CreateOp | ir.UpdateOp) & ir.ConsumesVarsTrait): n
128127
slots += op.expression.expressions.length;
129128
}
130129
return slots;
130+
case ir.OpKind.Control:
131+
// 1 for the [field] binding itself.
132+
// 1 for the control bindings object containing bound field states properties.
133+
return 2;
131134
case ir.OpKind.TwoWayProperty:
132135
// Two-way properties can only have expressions so they only need one variable slot.
133136
return 1;

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

Lines changed: 232 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,26 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88
import {RuntimeError, RuntimeErrorCode} from '../../errors';
9+
import {getClosureSafeProperty} from '../../util/property';
910
import {bindingUpdated} from '../bindings';
10-
import {ɵCONTROL, ɵControl} from '../interfaces/control';
11+
import {ɵCONTROL, ɵControl, ɵFieldState} from '../interfaces/control';
1112
import {ComponentDef} from '../interfaces/definition';
1213
import {InputFlags} from '../interfaces/input_flags';
1314
import {TElementNode, TNode, TNodeFlags, TNodeType} from '../interfaces/node';
1415
import {Renderer} from '../interfaces/renderer';
1516
import {SanitizerFn} from '../interfaces/sanitization';
1617
import {isComponentHost} from '../interfaces/type_checks';
1718
import {LView, RENDERER, TView} from '../interfaces/view';
18-
import {getCurrentTNode, getLView, getSelectedTNode, getTView, nextBindingIndex} from '../state';
19+
import {Signal} from '../reactivity/api';
20+
import {
21+
getBindingIndex,
22+
getCurrentTNode,
23+
getLView,
24+
getSelectedTNode,
25+
getTView,
26+
nextBindingIndex,
27+
} from '../state';
28+
import {NO_CHANGE} from '../tokens';
1929
import {isNameOnlyAttributeMarker} from '../util/attrs_utils';
2030
import {getNativeByTNode} from '../util/view_utils';
2131
import {listenToOutput} from '../view/directive_outputs';
@@ -91,6 +101,12 @@ export function ɵɵcontrol<T>(value: T, sanitizer?: SanitizerFn | null): void {
91101
updateNativeControl(tNode, lView, control);
92102
}
93103
}
104+
105+
// This instruction requires an additional variable slot to store control property bindings, but
106+
// may not use them if the `control` is undefined, so we increment the index here rather than when
107+
// used to ensure it happens unconditionally. Otherwise, the next instruction could begin with the
108+
// wrong binding index.
109+
nextBindingIndex();
94110
}
95111

96112
function getControlDirectiveFirstCreatePass<T>(
@@ -339,42 +355,40 @@ function updateCustomControl(
339355
const component = lView[componentIndex];
340356
const componentDef = tView.data[componentIndex] as ComponentDef<{}>;
341357
const state = control.state();
342-
// TODO: https://github.com/orgs/angular/projects/60/views/1?pane=issue&itemId=131711472
343-
// * check if bindings changed before writing.
344-
// * cache which inputs exist.
345-
writeToDirectiveInput(componentDef, component, modelName, state.value());
346-
maybeWriteToDirectiveInput(componentDef, component, 'errors', state.errors);
347-
maybeWriteToDirectiveInput(componentDef, component, 'invalid', state.invalid);
348-
maybeWriteToDirectiveInput(componentDef, component, 'disabled', state.disabled);
349-
maybeWriteToDirectiveInput(componentDef, component, 'disabledReasons', state.disabledReasons);
350-
maybeWriteToDirectiveInput(componentDef, component, 'name', state.name);
351-
maybeWriteToDirectiveInput(componentDef, component, 'readonly', state.readonly);
352-
maybeWriteToDirectiveInput(componentDef, component, 'touched', state.touched);
353-
354-
maybeWriteToDirectiveInput(componentDef, component, 'max', state.max);
355-
maybeWriteToDirectiveInput(componentDef, component, 'maxLength', state.maxLength);
356-
maybeWriteToDirectiveInput(componentDef, component, 'min', state.min);
357-
maybeWriteToDirectiveInput(componentDef, component, 'minLength', state.minLength);
358-
maybeWriteToDirectiveInput(componentDef, component, 'pattern', state.pattern);
359-
maybeWriteToDirectiveInput(componentDef, component, 'required', state.required);
358+
const bindings = getControlBindings(lView);
359+
360+
maybeUpdateInput(componentDef, component, bindings, state, VALUE, modelName);
361+
362+
for (const key of CONTROL_BINDING_KEYS) {
363+
const inputName = CONTROL_BINDING_NAMES[key];
364+
maybeUpdateInput(componentDef, component, bindings, state, key, inputName);
365+
}
360366
}
361367

362368
/**
363-
* Writes the specified value to a directive input if the input exists.
369+
* Binds a value from the field state to a component input, if the input exists and the value has
370+
* changed.
364371
*
365-
* @param componentDef The definition of the component that owns the input.
366-
* @param component The component instance.
367-
* @param inputName The name of the input to write to.
368-
* @param source A function that returns the value to write.
372+
* @param componentDef The component definition used to check for the input.
373+
* @param component The component instance to update.
374+
* @param bindings A map of previously bound values to check for changes.
375+
* @param state The control's field state.
376+
* @param key The key of the property in the `ɵFieldState` to bind.
377+
* @param inputName The name of the input to update.
369378
*/
370-
function maybeWriteToDirectiveInput(
379+
function maybeUpdateInput(
371380
componentDef: ComponentDef<unknown>,
372381
component: unknown,
382+
bindings: ControlBindings,
383+
state: ɵFieldState<unknown>,
384+
key: ControlBindingKeys,
373385
inputName: string,
374-
source?: () => unknown,
375-
) {
376-
if (source && inputName in componentDef.inputs) {
377-
writeToDirectiveInput(componentDef, component, inputName, source());
386+
): void {
387+
if (inputName in componentDef.inputs) {
388+
const value = state[key]?.();
389+
if (controlBindingUpdated(bindings, key, value)) {
390+
writeToDirectiveInput(componentDef, component, inputName, value);
391+
}
378392
}
379393
}
380394

@@ -386,37 +400,81 @@ function maybeWriteToDirectiveInput(
386400
* @param control The `ɵControl` directive instance.
387401
*/
388402
function updateNativeControl(tNode: TNode, lView: LView, control: ɵControl<unknown>): void {
389-
const input = getNativeByTNode(tNode, lView) as NativeControlElement;
403+
const element = getNativeByTNode(tNode, lView) as NativeControlElement;
390404
const renderer = lView[RENDERER];
391405
const state = control.state();
406+
const bindings = getControlBindings(lView);
392407

393-
// TODO: https://github.com/orgs/angular/projects/60/views/1?pane=issue&itemId=131711472
394-
// * check if bindings changed before writing.
395-
setNativeControlValue(input, state.value());
396-
renderer.setAttribute(input, 'name', state.name());
397-
setBooleanAttribute(renderer, input, 'disabled', state.disabled());
398-
setBooleanAttribute(renderer, input, 'readonly', state.readonly());
408+
const value = state.value();
409+
if (controlBindingUpdated(bindings, VALUE, value)) {
410+
setNativeControlValue(element, value);
411+
}
399412

400-
if (state.required) {
401-
setBooleanAttribute(renderer, input, 'required', state.required());
413+
const name = state.name();
414+
if (controlBindingUpdated(bindings, NAME, name)) {
415+
renderer.setAttribute(element, 'name', name);
402416
}
403417

418+
updateBooleanAttribute(renderer, element, bindings, state, DISABLED);
419+
updateBooleanAttribute(renderer, element, bindings, state, READONLY);
420+
updateBooleanAttribute(renderer, element, bindings, state, REQUIRED);
421+
404422
if (tNode.flags & TNodeFlags.isNativeNumericControl) {
405-
if (state.max) {
406-
setOptionalAttribute(renderer, input, 'max', state.max());
407-
}
408-
if (state.min) {
409-
setOptionalAttribute(renderer, input, 'min', state.min());
410-
}
423+
updateOptionalAttribute(renderer, element, bindings, state, MAX);
424+
updateOptionalAttribute(renderer, element, bindings, state, MIN);
411425
}
412426

413427
if (tNode.flags & TNodeFlags.isNativeTextControl) {
414-
if (state.maxLength) {
415-
setOptionalAttribute(renderer, input, 'maxLength', state.maxLength());
416-
}
417-
if (state.minLength) {
418-
setOptionalAttribute(renderer, input, 'minLength', state.minLength());
419-
}
428+
updateOptionalAttribute(renderer, element, bindings, state, MAX_LENGTH);
429+
updateOptionalAttribute(renderer, element, bindings, state, MIN_LENGTH);
430+
}
431+
}
432+
433+
/**
434+
* Binds a boolean property to a DOM attribute.
435+
*
436+
* @param renderer The renderer used to update the DOM.
437+
* @param element The element to update.
438+
* @param bindings The control bindings to check for changes.
439+
* @param state The control's field state.
440+
* @param key The key of the boolean property in the `ɵFieldState`.
441+
*/
442+
function updateBooleanAttribute(
443+
renderer: Renderer,
444+
element: HTMLElement,
445+
bindings: ControlBindings,
446+
state: ɵFieldState<unknown>,
447+
key: typeof DISABLED | typeof READONLY | typeof REQUIRED,
448+
) {
449+
const value = state[key]();
450+
if (controlBindingUpdated(bindings, key, value)) {
451+
const name = CONTROL_BINDING_NAMES[key];
452+
setBooleanAttribute(renderer, element, name, value);
453+
}
454+
}
455+
456+
/**
457+
* Binds a value source, if it exists, to an optional DOM attribute.
458+
*
459+
* An optional DOM attribute will be added, if defined, or removed, if undefined.
460+
*
461+
* @param renderer The renderer used to update the DOM.
462+
* @param element The element to update.
463+
* @param bindings The control bindings to check for changes.
464+
* @param state The control's field state.
465+
* @param key The key of the optional property in the `ɵFieldState`.
466+
*/
467+
function updateOptionalAttribute(
468+
renderer: Renderer,
469+
element: HTMLElement,
470+
bindings: ControlBindings,
471+
state: ɵFieldState<unknown>,
472+
key: typeof MAX | typeof MAX_LENGTH | typeof MIN | typeof MIN_LENGTH,
473+
): void {
474+
const value = state[key]?.();
475+
if (controlBindingUpdated(bindings, key, value)) {
476+
const name = CONTROL_BINDING_NAMES[key];
477+
setOptionalAttribute(renderer, element, name, value);
420478
}
421479
}
422480

@@ -457,7 +515,7 @@ function isNumericInput(tNode: TElementNode): boolean {
457515
}
458516

459517
/**
460-
* Returns whether `control` is a text-based input.
518+
* Returns whether `tNode` represents a text-based input.
461519
*
462520
* This is not the same as an input with `type="text"`, but rather any input that accepts
463521
* text-based input which includes numeric types.
@@ -558,6 +616,128 @@ function setNativeControlValue(element: NativeControlElement, value: unknown) {
558616
element.value = value as string;
559617
}
560618

619+
/** A property-renaming safe reference to a property named 'disabled'. */
620+
const DISABLED = /* @__PURE__ */ getClosureSafeProperty({
621+
disabled: getClosureSafeProperty,
622+
}) as 'disabled';
623+
624+
/** A property-renaming safe reference to a property named 'max'. */
625+
const MAX = /* @__PURE__ */ getClosureSafeProperty({max: getClosureSafeProperty}) as 'max';
626+
627+
/** A property-renaming safe reference to a property named 'maxLength'. */
628+
const MAX_LENGTH = /* @__PURE__ */ getClosureSafeProperty({
629+
maxLength: getClosureSafeProperty,
630+
}) as 'maxLength';
631+
632+
/** A property-renaming safe reference to a property named 'min'. */
633+
const MIN = /* @__PURE__ */ getClosureSafeProperty({min: getClosureSafeProperty}) as 'min';
634+
635+
/** A property-renaming safe reference to a property named 'minLength'. */
636+
const MIN_LENGTH = /* @__PURE__ */ getClosureSafeProperty({
637+
minLength: getClosureSafeProperty,
638+
}) as 'minLength';
639+
640+
/** A property-renaming safe reference to a property named 'name'. */
641+
const NAME = /* @__PURE__ */ getClosureSafeProperty({name: getClosureSafeProperty}) as 'name';
642+
643+
/** A property-renaming safe reference to a property named 'readonly'. */
644+
const READONLY = /* @__PURE__ */ getClosureSafeProperty({
645+
readonly: getClosureSafeProperty,
646+
}) as 'readonly';
647+
648+
/** A property-renaming safe reference to a property named 'required'. */
649+
const REQUIRED = /* @__PURE__ */ getClosureSafeProperty({
650+
required: getClosureSafeProperty,
651+
}) as 'required';
652+
653+
/** A property-renaming safe reference to a property named 'value'. */
654+
const VALUE = /* @__PURE__ */ getClosureSafeProperty({value: getClosureSafeProperty}) as 'value';
655+
656+
/**
657+
* A utility type that extracts the keys from `T` where the value type matches `TCondition`.
658+
* @template T The object type to extract keys from.
659+
* @template TCondition The condition to match the value type against.
660+
*/
661+
type KeysWithValueType<T, TCondition> = keyof {
662+
[K in keyof T as T[K] extends TCondition ? K : never]: never;
663+
};
664+
665+
/**
666+
* The keys of `ɵFieldState` that can be bound to a control.
667+
* These are the properties of `ɵFieldState` that are signals or undefined.
668+
*/
669+
type ControlBindingKeys = KeysWithValueType<ɵFieldState<unknown>, Signal<any> | undefined>;
670+
671+
/**
672+
* A map of control binding keys to their values.
673+
* Used to store the last seen values of bound control properties to check for changes.
674+
*/
675+
type ControlBindings = {
676+
[K in ControlBindingKeys]?: unknown;
677+
};
678+
679+
/**
680+
* A map of field state properties to control binding name.
681+
*
682+
* This excludes `value` whose corresponding control binding name differs between control types.
683+
*
684+
* The control binding name can be used for inputs or attributes (since DOM attributes are case
685+
* insensitive).
686+
*/
687+
const CONTROL_BINDING_NAMES = {
688+
disabled: 'disabled',
689+
disabledReasons: 'disabledReasons',
690+
errors: 'errors',
691+
invalid: 'invalid',
692+
max: 'max',
693+
maxLength: 'maxLength',
694+
min: 'min',
695+
minLength: 'minLength',
696+
name: 'name',
697+
pattern: 'pattern',
698+
readonly: 'readonly',
699+
required: 'required',
700+
touched: 'touched',
701+
} as const satisfies Record<Exclude<ControlBindingKeys, 'value'>, string>;
702+
703+
/** The keys of {@link CONTROL_BINDING_NAMES} */
704+
const CONTROL_BINDING_KEYS = /* @__PURE__ */ (() => Object.keys(CONTROL_BINDING_NAMES))() as Array<
705+
keyof typeof CONTROL_BINDING_NAMES
706+
>;
707+
708+
/**
709+
* Returns the values of field state properties bound to a control.
710+
*/
711+
function getControlBindings(lView: LView): ControlBindings {
712+
const bindingIndex = getBindingIndex();
713+
let bindings = lView[bindingIndex];
714+
if (bindings === NO_CHANGE) {
715+
bindings = lView[bindingIndex] = {};
716+
}
717+
return bindings;
718+
}
719+
720+
/**
721+
* Updates a control binding if changed, then returns whether it was updated.
722+
*
723+
* @param bindings The control bindings to check.
724+
* @param key The key of the binding to check.
725+
* @param value The new value to check against.
726+
* @returns `true` if the binding has changed.
727+
*/
728+
function controlBindingUpdated(
729+
bindings: ControlBindings,
730+
key: ControlBindingKeys,
731+
value: unknown,
732+
): boolean {
733+
const oldValue = bindings[key];
734+
if (Object.is(oldValue, value)) {
735+
return false;
736+
}
737+
bindings[key] = value;
738+
return true;
739+
}
740+
561741
/**
562742
* Sets a boolean attribute on an element.
563743
*

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ export interface ɵFieldState<T> {
114114
/**
115115
* A signal indicating the patterns the field must match.
116116
*/
117-
readonly pattern?: Signal<readonly RegExp[]>;
117+
readonly pattern: Signal<readonly RegExp[]>;
118118

119119
/**
120120
* A signal indicating whether the field is currently readonly.
@@ -124,7 +124,7 @@ export interface ɵFieldState<T> {
124124
/**
125125
* A signal indicating whether the field is required.
126126
*/
127-
readonly required?: Signal<boolean>;
127+
readonly required: Signal<boolean>;
128128

129129
/**
130130
* A signal indicating whether the field has been touched by the user.

0 commit comments

Comments
 (0)