Skip to content

fix(forms): fix some things that g3 complains about #63405

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

Merged
merged 2 commits into from
Aug 26, 2025
Merged

fix(forms): fix some things that g3 complains about #63405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1f1c6b8
fix(forms): fix some things that g3 complains about
mmalerba Aug 26, 2025
e8d06d7
fixup! fix(forms): fix some things that g3 complains about
mmalerba Aug 26, 2025
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
79 changes: 45 additions & 34 deletions packages/forms/signals/src/controls/control.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
OutputRef,
OutputRefSubscription,
reflectComponentType,
Renderer2,
signal,
Type,
untracked,
Expand All @@ -37,7 +38,7 @@ import {
PATTERN,
REQUIRED,
} from '../api/property';
import {Field} from '../api/types';
import type {Field} from '../api/types';
import type {FieldNode} from '../field/node';
import {
illegallyGetComponentInstance,
Expand Down Expand Up @@ -75,6 +76,7 @@ import {InteropNgControl} from './interop_ng_control';
export class Control<T> {
/** The injector for this component. */
private readonly injector = inject(Injector);
private readonly renderer = inject(Renderer2);

/** Whether state synchronization with the field has been setup yet. */
private initialized = false;
Expand Down Expand Up @@ -202,16 +204,25 @@ export class Control<T> {
});
input.addEventListener('blur', () => this.state().markAsTouched());

this.maybeSynchronize(() => this.state().readonly(), withBooleanAttribute(input, 'readonly'));
this.maybeSynchronize(
() => this.state().readonly(),
this.withBooleanAttribute(input, 'readonly'),
);
// TODO: consider making a global configuration option for using aria-disabled instead.
this.maybeSynchronize(() => this.state().disabled(), withBooleanAttribute(input, 'disabled'));
this.maybeSynchronize(() => this.state().name(), withAttribute(input, 'name'));
this.maybeSynchronize(
() => this.state().disabled(),
this.withBooleanAttribute(input, 'disabled'),
);
this.maybeSynchronize(() => this.state().name(), this.withAttribute(input, 'name'));

this.maybeSynchronize(this.propertySource(REQUIRED), withBooleanAttribute(input, 'required'));
this.maybeSynchronize(this.propertySource(MIN), withAttribute(input, 'min'));
this.maybeSynchronize(this.propertySource(MIN_LENGTH), withAttribute(input, 'minLength'));
this.maybeSynchronize(this.propertySource(MAX), withAttribute(input, 'max'));
this.maybeSynchronize(this.propertySource(MAX_LENGTH), withAttribute(input, 'maxLength'));
this.maybeSynchronize(
this.propertySource(REQUIRED),
this.withBooleanAttribute(input, 'required'),
);
this.maybeSynchronize(this.propertySource(MIN), this.withAttribute(input, 'min'));
this.maybeSynchronize(this.propertySource(MIN_LENGTH), this.withAttribute(input, 'minLength'));
this.maybeSynchronize(this.propertySource(MAX), this.withAttribute(input, 'max'));
this.maybeSynchronize(this.propertySource(MAX_LENGTH), this.withAttribute(input, 'maxLength'));

switch (inputType) {
case 'checkbox':
Expand Down Expand Up @@ -360,38 +371,38 @@ export class Control<T> {
);
return () => metaSource()?.();
}

/** Creates a (non-boolean) value sync that writes the given attribute of the given element. */
private withAttribute(
element: HTMLElement,
attribute: string,
): (value: {toString(): string} | undefined) => void {
return (value) => {
if (value !== undefined) {
this.renderer.setAttribute(element, attribute, value.toString());
} else {
this.renderer.removeAttribute(element, attribute);
}
};
}

/** Creates a boolean value sync that writes the given attribute of the given element. */
private withBooleanAttribute(element: HTMLElement, attribute: string): (value: boolean) => void {
return (value) => {
if (value) {
this.renderer.setAttribute(element, attribute, '');
} else {
this.renderer.removeAttribute(element, attribute);
}
};
}
}

/** Creates a value sync from an input signal. */
function withInput<T>(input: InputSignal<T> | undefined): ((value: T) => void) | undefined {
return input ? (value: T) => illegallySetInputSignal(input, value) : undefined;
}

/** Creates a boolean value sync that writes the given attribute of the given element. */
function withBooleanAttribute(element: HTMLElement, attribute: string): (value: boolean) => void {
return (value) => {
if (value) {
element.setAttribute(attribute, '');
} else {
element.removeAttribute(attribute);
}
};
}

/** Creates a (non-boolean) value sync that writes the given attribute of the given element. */
function withAttribute(
element: HTMLElement,
attribute: string,
): (value: {toString(): string} | undefined) => void {
return (value) => {
if (value !== undefined) {
element.setAttribute(attribute, value.toString());
} else {
element.removeAttribute(attribute);
}
};
}

/**
* Checks whether the given component matches the contract for either FormValueControl or
* FormCheckboxControl.
Expand Down
4 changes: 2 additions & 2 deletions packages/forms/signals/src/field/context.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {Field, FieldContext, FieldPath, FieldState} from '../api/types';
import {FieldPathNode} from '../schema/path_node';
import {isArray} from '../util/type_guards';
import type {FieldNode} from './node';
import {boundPathDepth} from './resolution';
import {getBoundPathDepth} from './resolution';

/**
* `FieldContext` implementation, backed by a `FieldNode`.
Expand Down Expand Up @@ -49,7 +49,7 @@ export class FieldNodeContext implements FieldContext<unknown> {
// This ensures that we do not accidentally match on the wrong application of a recursively
// applied schema.
let field: FieldNode | undefined = this.node;
let stepsRemaining = boundPathDepth;
let stepsRemaining = getBoundPathDepth();
while (stepsRemaining > 0 || !field.structure.logic.hasLogic(targetPathNode.root.logic)) {
stepsRemaining--;
field = field.structure.parent;
Expand Down
6 changes: 5 additions & 1 deletion packages/forms/signals/src/field/resolution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,15 @@
* found in the LICENSE file at https://angular.dev/license
*/

let boundPathDepth = 0;

/**
* The depth of the current path when evaluating a logic function.
* Do not set this directly, it is a context variable managed by `setBoundPathDepthForResolution`.
*/
export let boundPathDepth = 0;
export function getBoundPathDepth() {
return boundPathDepth;
}

/**
* Sets the bound path depth for the duration of the given logic function.
Expand Down
Loading