Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -2923,6 +2923,22 @@ describe('type check blocks', () => {
expect(block).toContain('_t2.field = (((this).f));');
});

it('should generate a string array field for a multiple select', () => {
const block = tcb('<select multiple [formField]="f"></select>', [FieldMock]);
expect(block).toContain('var _t1 = null! as string[];');
expect(block).toContain('_t1 = ((this).f)().value();');
expect(block).toContain('var _t2 = null! as i0.FormField;');
expect(block).toContain('_t2.field = (((this).f));');
});

it('should generate a union type for a select with a dynamic [multiple] binding', () => {
const block = tcb('<select [multiple]="isDynamic" [formField]="f"></select>', [FieldMock]);
expect(block).toContain('var _t1 = null! as string | string[];');
expect(block).toContain('_t1 = ((this).f)().value();');
expect(block).toContain('var _t2 = null! as i0.FormField;');
expect(block).toContain('_t2.field = (((this).f));');
});

it('should generate a custom value control', () => {
const block = tcb('<custom-control [formField]="f"/>', [
FieldMock,
Expand Down
27 changes: 25 additions & 2 deletions packages/compiler/src/typecheck/ops/signal_forms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,15 @@ export class TcbNativeFieldOp extends TcbOp {
}

private getExpectedTypeFromDomNode(node: Element): string | null {
if (node.name === 'textarea' || node.name === 'select') {
// `<textarea>` and `<select>` are always strings.
if (node.name === 'textarea') {
// `<textarea>` is always a string.
return 'string';
}

if (node.name === 'select') {
const mode = getSelectMultipleMode(node);
if (mode === 'static') return 'string[]';
if (mode === 'dynamic') return 'string | string[]';
return 'string';
}

Expand Down Expand Up @@ -196,6 +203,22 @@ export class TcbNativeFieldOp extends TcbOp {
}
}

function getSelectMultipleMode(node: Element): 'static' | 'dynamic' | 'none' {
if (node.attributes.some((attr) => attr.name.toLowerCase() === 'multiple')) {
return 'static';
}
if (
node.inputs.some(
(input) =>
(input.type === BindingType.Property || input.type === BindingType.Attribute) &&
input.name.toLowerCase() === 'multiple',
)
) {
return 'dynamic';
}
return 'none';
}

/**
* A variation of the `TcbNativeFieldOp` with specific logic for radio buttons.
*/
Expand Down
45 changes: 45 additions & 0 deletions packages/forms/signals/src/directive/bindings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
* found in the LICENSE file at https://angular.dev/license
*/

import {type ɵControlDirectiveHost as ControlDirectiveHost, type Renderer2} from '@angular/core';
import type {ReadonlyFieldState} from '../api/types';
import {setNativeDomProperty, type NativeFormControl} from './native';

/**
* Branded type for the public name of an input we bind on control components or DOM elements.
Expand Down Expand Up @@ -67,6 +69,22 @@ export function readFieldStateBindingValue(
export const CONTROL_BINDING_NAMES = /* @__PURE__ */ (() =>
Object.values(FIELD_STATE_KEY_TO_CONTROL_BINDING))() as Array<ControlBindingKey>;

/** The subset of native property names that Signal Forms can write via the Renderer. */
type NativeDomPropertyName = Parameters<typeof setNativeDomProperty>[2];

/**
* Structural interface describing the pieces of `FormField` that `applyControlStateBindings`
* needs. Using a structural type avoids a circular import between `bindings.ts` and
* `form_field.ts`.
*/
export interface ControlStateBindingTarget {
readonly renderer: Renderer2;
readonly nativeFormElement: NativeFormControl;
elementAcceptsNativeProperty(
name: ControlBindingKey,
): name is ControlBindingKey & NativeDomPropertyName;
}

export function createBindings<TKey extends string>(): {[K in TKey]?: unknown} {
return {};
}
Expand All @@ -82,3 +100,30 @@ export function bindingUpdated<TKey extends string>(
}
return false;
}

/**
* Iterates over all control binding names, computes each value from field state, and applies
* any changed bindings to both the directive inputs and the native DOM element. Shared by
* native and select-multiple control implementations.
*/
export function applyControlStateBindings(
bindings: {[key: string]: unknown},
state: ReadonlyFieldState<unknown>,
host: ControlDirectiveHost,
parent: ControlStateBindingTarget,
): void {
for (const name of CONTROL_BINDING_NAMES) {
const value = readFieldStateBindingValue(state, name);
if (bindingUpdated(bindings, name, value)) {
host.setInputOnDirectives(name, value);
if (parent.elementAcceptsNativeProperty(name)) {
setNativeDomProperty(
parent.renderer,
parent.nativeFormElement,
name,
value as string | number | undefined,
);
}
}
}
}
31 changes: 19 additions & 12 deletions packages/forms/signals/src/directive/control_native.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,18 @@
* found in the LICENSE file at https://angular.dev/license
*/
import {
ɵRuntimeError as RuntimeError,
type ɵControlDirectiveHost as ControlDirectiveHost,
type Signal,
type WritableSignal,
} from '@angular/core';
import type {ValidationError} from '../api/rules';
import {RuntimeErrorCode} from '../errors';
import {createParser} from '../util/parser';
import {
applyControlStateBindings,
bindingUpdated,
CONTROL_BINDING_NAMES,
createBindings,
readFieldStateBindingValue,
type ControlBindingKey,
} from './bindings';
import type {FormField} from './form_field';
Expand All @@ -26,7 +27,6 @@ import {
isInput,
inputRequiresValidityTracking,
setNativeControlValue,
setNativeDomProperty,
} from './native';
import {observeSelectMutations} from './select';

Expand Down Expand Up @@ -90,21 +90,28 @@ export function nativeControlCreate(
const bindings = createBindings<ControlBindingKey | 'controlValue'>();

return () => {
if (
ngDevMode &&
!updateMode &&
input.tagName === 'SELECT' &&
(input as HTMLSelectElement).multiple
) {
throw new RuntimeError(
RuntimeErrorCode.DYNAMIC_SELECT_MULTIPLE_BINDING,
ngDevMode &&
`Signal Forms does not support dynamic [multiple] bindings on <select>. ` +
`Use the static 'multiple' attribute instead.`,
);
}

const state = parent.state();
const controlValue = state.controlValue();

if (bindingUpdated(bindings, 'controlValue', controlValue)) {
setNativeControlValue(input, controlValue);
}

for (const name of CONTROL_BINDING_NAMES) {
const value = readFieldStateBindingValue(state, name);
if (bindingUpdated(bindings, name, value)) {
host.setInputOnDirectives(name, value);
if (parent.elementAcceptsNativeProperty(name)) {
setNativeDomProperty(parent.renderer, input, name, value as string | number | undefined);
}
}
}
applyControlStateBindings(bindings, state, host, parent);

updateMode = true;
};
Expand Down
96 changes: 96 additions & 0 deletions packages/forms/signals/src/directive/control_select_multiple.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.dev/license
*/
import {
ɵRuntimeError as RuntimeError,
type ɵControlDirectiveHost as ControlDirectiveHost,
} from '@angular/core';
import {RuntimeErrorCode} from '../errors';
import {
applyControlStateBindings,
bindingUpdated,
createBindings,
type ControlBindingKey,
} from './bindings';
import type {FormField} from './form_field';
import {observeSelectMutations} from './select';

export function selectMultipleControlCreate(
Comment thread
JeanMeche marked this conversation as resolved.
host: ControlDirectiveHost,
parent: FormField<unknown>,
): () => void {
let updateMode = false;
const select = parent.nativeFormElement as HTMLSelectElement;

host.listenToDom('input', () =>
parent.state().controlValue.set(getSelectMultipleControlValue(select)),
);
host.listenToDom('blur', () => parent.state().markAsTouched());

parent.registerAsBinding();

observeSelectMutations(
select,
() => {
if (!updateMode) {
return;
}
setSelectMultipleControlValue(select, parent.state().controlValue());
},
parent.destroyRef,
);

const bindings = createBindings<ControlBindingKey | 'controlValue'>();

return () => {
if (ngDevMode && !updateMode && !select.multiple) {
throw new RuntimeError(
RuntimeErrorCode.DYNAMIC_SELECT_MULTIPLE_BINDING,
ngDevMode &&
`Signal Forms does not support dynamic [multiple] bindings on <select>. ` +
`Use the static 'multiple' attribute instead.`,
);
}

const state = parent.state();
const controlValue = state.controlValue();
if (bindingUpdated(bindings, 'controlValue', controlValue)) {
setSelectMultipleControlValue(select, controlValue);
}

applyControlStateBindings(bindings, state, host, parent);

updateMode = true;
};
}

function getSelectMultipleControlValue(select: HTMLSelectElement): string[] {
const selected: string[] = [];
for (let i = 0; i < select.options.length; i++) {
const option = select.options[i];
if (option.selected) {
selected.push(option.value);
}
}
return selected;
}
Comment on lines +71 to +80

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not obvious why we need to cover both cases. Can you add a comment about that

@sonukapoor sonukapoor Apr 27, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment to the fallback branch explaining why it exists. selectedOptions isn't available in all environments (older jsdom in particular), so we iterate options and check the selected flag as a fallback.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

older jsdom in particular

Angular doesn't use JSDOM but Domino. But it looks like this argument still hold. I couldn't find any trace of selectedOptions on https://github.com/angular/domino.

@sonukapoor sonukapoor Apr 27, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good correction, thanks. It's Domino, not jsdom. I updated the comment to reflect that.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I second guessing this. If we have that 2nd part that checks for selected. Do we actually need selectedOptions ?

@sonukapoor sonukapoor Apr 27, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The options/selected fallback is functionally equivalent and works in all environments, so selectedOptions was only ever a minor performance optimization. For typical multi-selects the difference is negligible. Dropped the selectedOptions path entirely and simplified to a single loop.


function setSelectMultipleControlValue(select: HTMLSelectElement, value: unknown): void {
if (!Array.isArray(value)) {
throw new RuntimeError(
RuntimeErrorCode.SELECT_MULTIPLE_NON_ARRAY_VALUE,
ngDevMode &&
`Expected an array value for select[multiple], but got ${typeof value}. ` +
`Bind a signal holding a string array: e.g. form(signal<string[]>([]))`,
);
}
const selectedValues = new Set<string>(value);
for (let i = 0; i < select.options.length; i++) {
const option = select.options[i];
option.selected = selectedValues.has(option.value);
}
}
7 changes: 7 additions & 0 deletions packages/forms/signals/src/directive/form_field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {bindingUpdated, type ControlBindingKey, createBindings} from './bindings
import {customControlCreate} from './control_custom';
import {cvaControlCreate} from './control_cva';
import {nativeControlCreate} from './control_native';
import {selectMultipleControlCreate} from './control_select_multiple';
import {
isNativeFormElement,
isNumericFormElement,
Expand Down Expand Up @@ -341,6 +342,12 @@ export class FormField<T> {
this.ɵngControlUpdate = cvaControlCreate(host, this as FormField<unknown>);
} else if (host.customControl) {
this.ɵngControlUpdate = customControlCreate(host, this as FormField<unknown>);
} else if (
this.elementIsNativeFormElement &&
this.nativeFormElement.tagName === 'SELECT' &&
(this.nativeFormElement as HTMLSelectElement).multiple

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same concern raised with the type check code above. If the element has a [multiple] property binding, it may not be set yet at this point. Do we want to delegate all <select> elements through a selectControlCreate(), which dynamically handles multiple? Alternatively we can prohibit a dynamic multiple binding if we prefer.

@sonukapoor sonukapoor Apr 28, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went with the prohibit option. The check happens at the start of the first update cycle (after Angular has applied property bindings), so it catches the case where [multiple] was set dynamically before ɵngControlCreate had a chance to see the correct value. If select.multiple doesn't match what was assumed at creation time, a DYNAMIC_SELECT_MULTIPLE_BINDING error is thrown in dev mode pointing to the static attribute as the fix. A unified dynamic handler could be a follow-up if there's demand for it.

) {
this.ɵngControlUpdate = selectMultipleControlCreate(host, this as FormField<unknown>);
} else if (this.elementIsNativeFormElement) {
this.ɵngControlUpdate = nativeControlCreate(
host,
Expand Down
2 changes: 2 additions & 0 deletions packages/forms/signals/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,6 @@ export const enum RuntimeErrorCode {
MISSING_SUBMIT_ACTION = 1915,
RENDERED_HIDDEN_FIELD = 1916,
UNSUPPORTED_FEATURE = 1920,
SELECT_MULTIPLE_NON_ARRAY_VALUE = 1921,
DYNAMIC_SELECT_MULTIPLE_BINDING = 1922,
}
36 changes: 36 additions & 0 deletions packages/forms/signals/test/web/form_field.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3923,6 +3923,42 @@ describe('field directive', () => {
expect(cmp.f().value()).toBe('two');
});

it('synchronizes with a multiple select', () => {
@Component({
imports: [FormField],
template: `
<select #select multiple [formField]="f">
<option value="one">One</option>
<option value="two">Two</option>
<option value="three">Three</option>
</select>
`,
})
class TestCmp {
f = form(signal<string[]>(['two']));
select = viewChild.required<ElementRef<HTMLSelectElement>>('select');
}

const fix = act(() => TestBed.createComponent(TestCmp));
const select = fix.componentInstance.select().nativeElement;
const cmp = fix.componentInstance as TestCmp;

expect(Array.from(select.selectedOptions, (option) => option.value)).toEqual(['two']);

// Model -> View
act(() => cmp.f().value.set(['one', 'three']));
expect(Array.from(select.selectedOptions, (option) => option.value)).toEqual(['one', 'three']);

// View -> Model
act(() => {
for (const option of Array.from(select.options)) {
option.selected = option.value !== 'three';
}
select.dispatchEvent(new Event('input'));
});
expect(cmp.f().value()).toEqual(['one', 'two']);
});

it('synchronizes with a custom value control', () => {
@Component({
selector: 'my-input',
Expand Down
Loading