Skip to content

Commit fd9af2a

Browse files
leonsenftthePunderWoman
leonsenft
authored and
thePunderWoman
committed
fix(forms): only propagate schema defined properties from field to control (#64446)
Prior to this change, `FieldState` defined a signal for each built-in property. This unfortunately meant that the `Field` directive had no way of knowing which property had actually been defined in the schema, and would thus attempt to propagate them all to the bound form control. This meant that the default values of these signals would override the default or template defined values of these control properties. Now these properties are `undefined` by default, and only initialized if defined in the schema. Thus the `Field` directive will not attempt to bind any properties that aren't explicitly managed by the schema. PR Close #64446
1 parent 4261a8c commit fd9af2a

File tree

6 files changed

+289
-49
lines changed

6 files changed

+289
-49
lines changed

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

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -347,15 +347,16 @@ function updateCustomControl(
347347
maybeWriteToDirectiveInput(componentDef, component, 'invalid', state.invalid);
348348
maybeWriteToDirectiveInput(componentDef, component, 'disabled', state.disabled);
349349
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+
350354
maybeWriteToDirectiveInput(componentDef, component, 'max', state.max);
351355
maybeWriteToDirectiveInput(componentDef, component, 'maxLength', state.maxLength);
352356
maybeWriteToDirectiveInput(componentDef, component, 'min', state.min);
353357
maybeWriteToDirectiveInput(componentDef, component, 'minLength', state.minLength);
354-
maybeWriteToDirectiveInput(componentDef, component, 'name', state.name);
355358
maybeWriteToDirectiveInput(componentDef, component, 'pattern', state.pattern);
356-
maybeWriteToDirectiveInput(componentDef, component, 'readonly', state.readonly);
357359
maybeWriteToDirectiveInput(componentDef, component, 'required', state.required);
358-
maybeWriteToDirectiveInput(componentDef, component, 'touched', state.touched);
359360
}
360361

361362
/**
@@ -370,9 +371,9 @@ function maybeWriteToDirectiveInput(
370371
componentDef: ComponentDef<unknown>,
371372
component: unknown,
372373
inputName: string,
373-
source: () => unknown,
374+
source?: () => unknown,
374375
) {
375-
if (inputName in componentDef.inputs) {
376+
if (source && inputName in componentDef.inputs) {
376377
writeToDirectiveInput(componentDef, component, inputName, source());
377378
}
378379
}
@@ -395,16 +396,27 @@ function updateNativeControl(tNode: TNode, lView: LView, control: ɵControl<unkn
395396
renderer.setAttribute(input, 'name', state.name());
396397
setBooleanAttribute(renderer, input, 'disabled', state.disabled());
397398
setBooleanAttribute(renderer, input, 'readonly', state.readonly());
398-
setBooleanAttribute(renderer, input, 'required', state.required());
399+
400+
if (state.required) {
401+
setBooleanAttribute(renderer, input, 'required', state.required());
402+
}
399403

400404
if (tNode.flags & TNodeFlags.isNativeNumericControl) {
401-
setOptionalAttribute(renderer, input, 'max', state.max());
402-
setOptionalAttribute(renderer, input, 'min', state.min());
405+
if (state.max) {
406+
setOptionalAttribute(renderer, input, 'max', state.max());
407+
}
408+
if (state.min) {
409+
setOptionalAttribute(renderer, input, 'min', state.min());
410+
}
403411
}
404412

405413
if (tNode.flags & TNodeFlags.isNativeTextControl) {
406-
setOptionalAttribute(renderer, input, 'maxLength', state.maxLength());
407-
setOptionalAttribute(renderer, input, 'minLength', state.minLength());
414+
if (state.maxLength) {
415+
setOptionalAttribute(renderer, input, 'maxLength', state.maxLength());
416+
}
417+
if (state.minLength) {
418+
setOptionalAttribute(renderer, input, 'minLength', state.minLength());
419+
}
408420
}
409421
}
410422

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,28 +83,28 @@ export interface ɵFieldState<T> {
8383
*
8484
* Applies to `<input>` with a numeric or date `type` attribute and custom controls.
8585
*/
86-
readonly max: Signal<number | undefined>;
86+
readonly max?: Signal<number | undefined>;
8787

8888
/**
8989
* A signal indicating the field's maximum string length, if applicable.
9090
*
9191
* Applies to `<input>`, `<textarea>`, and custom controls.
9292
*/
93-
readonly maxLength: Signal<number | undefined>;
93+
readonly maxLength?: Signal<number | undefined>;
9494

9595
/**
9696
* A signal indicating the field's minimum value, if applicable.
9797
*
9898
* Applies to `<input>` with a numeric or date `type` attribute and custom controls.
9999
*/
100-
readonly min: Signal<number | undefined>;
100+
readonly min?: Signal<number | undefined>;
101101

102102
/**
103103
* A signal indicating the field's minimum string length, if applicable.
104104
*
105105
* Applies to `<input>`, `<textarea>`, and custom controls.
106106
*/
107-
readonly minLength: Signal<number | undefined>;
107+
readonly minLength?: Signal<number | undefined>;
108108

109109
/**
110110
* A signal of a unique name for the field, by default based on the name of its parent field.
@@ -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.

‎packages/forms/signals/src/field/node.ts‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -146,36 +146,40 @@ export class FieldNode implements FieldState<unknown> {
146146
return this.nodeState.name;
147147
}
148148

149-
get max(): Signal<number | undefined> {
150-
return this.property(MAX);
149+
private propertyOrUndefined<M>(prop: AggregateProperty<M, any>): Signal<M> | undefined {
150+
return this.hasProperty(prop) ? this.property(prop) : undefined;
151151
}
152152

153-
get maxLength(): Signal<number | undefined> {
154-
return this.property(MAX_LENGTH);
153+
get max(): Signal<number | undefined> | undefined {
154+
return this.propertyOrUndefined(MAX);
155155
}
156156

157-
get min(): Signal<number | undefined> {
158-
return this.property(MIN);
157+
get maxLength(): Signal<number | undefined> | undefined {
158+
return this.propertyOrUndefined(MAX_LENGTH);
159159
}
160160

161-
get minLength(): Signal<number | undefined> {
162-
return this.property(MIN_LENGTH);
161+
get min(): Signal<number | undefined> | undefined {
162+
return this.propertyOrUndefined(MIN);
163163
}
164164

165-
get pattern(): Signal<readonly RegExp[]> {
166-
return this.property(PATTERN);
165+
get minLength(): Signal<number | undefined> | undefined {
166+
return this.propertyOrUndefined(MIN_LENGTH);
167167
}
168168

169-
get required(): Signal<boolean> {
170-
return this.property(REQUIRED);
169+
get pattern(): Signal<readonly RegExp[]> | undefined {
170+
return this.propertyOrUndefined(PATTERN);
171+
}
172+
173+
get required(): Signal<boolean> | undefined {
174+
return this.propertyOrUndefined(REQUIRED);
171175
}
172176

173177
property<M>(prop: AggregateProperty<M, any>): Signal<M>;
174178
property<M>(prop: Property<M>): M | undefined;
175179
property<M>(prop: Property<M> | AggregateProperty<M, any>): Signal<M> | M | undefined {
176180
return this.propertyState.get(prop);
177181
}
178-
hasProperty(prop: Property<unknown> | AggregateProperty<unknown, any>): boolean {
182+
hasProperty(prop: Property<any> | AggregateProperty<any, any>): boolean {
179183
return this.propertyState.has(prop);
180184
}
181185

‎packages/forms/signals/src/field/property.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ export class FieldPropertyState {
5959
* @param prop
6060
* @returns
6161
*/
62-
has(prop: Property<unknown> | AggregateProperty<unknown, unknown>): boolean {
62+
has(prop: Property<any> | AggregateProperty<any, any>): boolean {
6363
if (prop instanceof AggregateProperty) {
6464
// For aggregate properties, they get added to the map lazily, on first access, so we can't
6565
// rely on checking presence in the properties map. Instead we check if there is any logic for

‎packages/forms/signals/src/schema/logic.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ export class LogicContainer {
284284
}
285285

286286
/** Checks whether there is logic for the given aggregate property. */
287-
hasAggregateProperty(prop: AggregateProperty<unknown, unknown>) {
287+
hasAggregateProperty(prop: AggregateProperty<any, any>) {
288288
return this.aggregateProperties.has(prop);
289289
}
290290

0 commit comments

Comments
 (0)