Skip to content

Commit c075ef1

Browse files
sonukapoorsonukapoor
committed
fix(forms): parse numeric model values for signal forms <select>
When a `<select>` element is bound via `[formField]` to a field with a `number | null` model, the native DOM always reads `element.value` as a string. This caused the model to silently receive a string (e.g. `"42"`) instead of the expected number, and writing `null` to `element.value` coerced it to the string `"null"` rather than clearing the selection. Extend `getNativeControlValue` with a `select-one` case that mirrors the existing `<input type="text">` numeric-model logic: when the current model type is `number | null`, parse the selected option's string value as a number, return `null` for an empty selection, and produce a parse error if the option value is not a valid number. Extend `setNativeControlValue` with a matching `select-one` case that converts `null` and `NaN` to `''` (clearing the selection) and writes numeric values as `String(value)`. Replace the bare `input.value = ... as string` assignment in the `observeSelectMutations` callback in `nativeControlCreate` with a call to `setNativeControlValue` so that option-change re-sync also benefits from the same null/number handling. Fixes angular#68217
1 parent 08930e6 commit c075ef1

3 files changed

Lines changed: 229 additions & 1 deletion

File tree

‎packages/forms/signals/src/directive/control_native.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ export function nativeControlCreate(
8181
if (!updateMode) {
8282
return;
8383
}
84-
input.value = parent.state().controlValue() as string;
84+
setNativeControlValue(input, parent.state().controlValue());
8585
},
8686
parent.destroyRef,
8787
);

‎packages/forms/signals/src/directive/native.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,21 @@ export function getNativeControlValue(
7474
return {value: element.valueAsNumber};
7575
}
7676
break;
77+
case 'select-one':
78+
// We can read a `number` or a `string` from a <select>. Prefer whichever is consistent
79+
// with the current type.
80+
modelValue = untracked(currentValue);
81+
if (typeof modelValue === 'number' || modelValue === null) {
82+
if (element.value === '') {
83+
return {value: null};
84+
}
85+
const parsed = Number(element.value);
86+
if (Number.isNaN(parsed)) {
87+
return {error: new NativeInputParseError() as WithoutFieldTree<NativeInputParseError>};
88+
}
89+
return {value: parsed};
90+
}
91+
break;
7792
}
7893

7994
// For text-like <input> elements, parse numeric values if the model is numeric.
@@ -136,6 +151,17 @@ export function setNativeControlValue(element: NativeFormControl, value: unknown
136151
setNativeNumberControlValue(element, value);
137152
return;
138153
}
154+
break;
155+
case 'select-one':
156+
// This input type can receive a `number`, `null`, or `string`.
157+
if (typeof value === 'number') {
158+
element.value = isNaN(value) ? '' : String(value);
159+
return;
160+
} else if (value === null) {
161+
element.value = '';
162+
return;
163+
}
164+
break;
139165
}
140166

141167
// For text-like <input> elements, handle numeric and null values.

‎packages/forms/signals/test/web/select_numeric.spec.ts‎

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.dev/license
7+
*/
8+
9+
import {Component, signal} from '@angular/core';
10+
import {TestBed} from '@angular/core/testing';
11+
import {FormField, form} from '../../public_api';
12+
13+
describe('select with numeric model', () => {
14+
it('should render initial number value by selecting the matching option', () => {
15+
@Component({
16+
imports: [FormField],
17+
template: `
18+
<select [formField]="f">
19+
<option value="1">One</option>
20+
<option value="2">Two</option>
21+
<option value="3">Three</option>
22+
</select>
23+
`,
24+
})
25+
class TestCmp {
26+
readonly data = signal<number>(2);
27+
readonly f = form(this.data);
28+
}
29+
30+
const fixture = act(() => TestBed.createComponent(TestCmp));
31+
const select = fixture.nativeElement.querySelector('select') as HTMLSelectElement;
32+
33+
expect(select.value).toBe('2');
34+
expect(fixture.componentInstance.f().value()).toBe(2);
35+
});
36+
37+
it('should update model as a number when user selects an option', () => {
38+
@Component({
39+
imports: [FormField],
40+
template: `
41+
<select [formField]="f">
42+
<option value="1">One</option>
43+
<option value="2">Two</option>
44+
<option value="3">Three</option>
45+
</select>
46+
`,
47+
})
48+
class TestCmp {
49+
readonly data = signal<number>(1);
50+
readonly f = form(this.data);
51+
}
52+
53+
const fixture = act(() => TestBed.createComponent(TestCmp));
54+
const select = fixture.nativeElement.querySelector('select') as HTMLSelectElement;
55+
56+
act(() => {
57+
select.value = '3';
58+
select.dispatchEvent(new Event('input'));
59+
});
60+
61+
expect(fixture.componentInstance.f().value()).toBe(3);
62+
expect(typeof fixture.componentInstance.f().value()).toBe('number');
63+
expect(fixture.componentInstance.f().errors()).toEqual([]);
64+
});
65+
66+
it('should render null model value as empty string', () => {
67+
@Component({
68+
imports: [FormField],
69+
template: `
70+
<select [formField]="f">
71+
<option value="">-- select --</option>
72+
<option value="1">One</option>
73+
<option value="2">Two</option>
74+
</select>
75+
`,
76+
})
77+
class TestCmp {
78+
readonly data = signal<number | null>(null);
79+
readonly f = form(this.data);
80+
}
81+
82+
const fixture = act(() => TestBed.createComponent(TestCmp));
83+
const select = fixture.nativeElement.querySelector('select') as HTMLSelectElement;
84+
85+
expect(select.value).toBe('');
86+
expect(fixture.componentInstance.f().value()).toBeNull();
87+
});
88+
89+
it('should update model to null when user selects the empty option', () => {
90+
@Component({
91+
imports: [FormField],
92+
template: `
93+
<select [formField]="f">
94+
<option value="">-- select --</option>
95+
<option value="1">One</option>
96+
<option value="2">Two</option>
97+
</select>
98+
`,
99+
})
100+
class TestCmp {
101+
readonly data = signal<number | null>(1);
102+
readonly f = form(this.data);
103+
}
104+
105+
const fixture = act(() => TestBed.createComponent(TestCmp));
106+
const select = fixture.nativeElement.querySelector('select') as HTMLSelectElement;
107+
108+
act(() => {
109+
select.value = '';
110+
select.dispatchEvent(new Event('input'));
111+
});
112+
113+
expect(fixture.componentInstance.f().value()).toBeNull();
114+
expect(fixture.componentInstance.f().errors()).toEqual([]);
115+
});
116+
117+
it('should update select when model is set programmatically', () => {
118+
@Component({
119+
imports: [FormField],
120+
template: `
121+
<select [formField]="f">
122+
<option value="10">Ten</option>
123+
<option value="20">Twenty</option>
124+
<option value="30">Thirty</option>
125+
</select>
126+
`,
127+
})
128+
class TestCmp {
129+
readonly data = signal<number>(10);
130+
readonly f = form(this.data);
131+
}
132+
133+
const fixture = act(() => TestBed.createComponent(TestCmp));
134+
const select = fixture.nativeElement.querySelector('select') as HTMLSelectElement;
135+
136+
expect(select.value).toBe('10');
137+
138+
act(() => {
139+
fixture.componentInstance.data.set(30);
140+
});
141+
142+
expect(select.value).toBe('30');
143+
expect(fixture.componentInstance.f().value()).toBe(30);
144+
});
145+
146+
it('should render NaN model value as empty string', () => {
147+
@Component({
148+
imports: [FormField],
149+
template: `
150+
<select [formField]="f">
151+
<option value="">-- select --</option>
152+
<option value="1">One</option>
153+
</select>
154+
`,
155+
})
156+
class TestCmp {
157+
readonly data = signal<number | null>(NaN);
158+
readonly f = form(this.data);
159+
}
160+
161+
const fixture = act(() => TestBed.createComponent(TestCmp));
162+
const select = fixture.nativeElement.querySelector('select') as HTMLSelectElement;
163+
164+
expect(select.value).toBe('');
165+
expect(fixture.componentInstance.f().value()).toEqual(NaN);
166+
});
167+
168+
it('should preserve string model type when model is a string', () => {
169+
@Component({
170+
imports: [FormField],
171+
template: `
172+
<select [formField]="f">
173+
<option value="a">A</option>
174+
<option value="b">B</option>
175+
</select>
176+
`,
177+
})
178+
class TestCmp {
179+
readonly data = signal<string>('a');
180+
readonly f = form(this.data);
181+
}
182+
183+
const fixture = act(() => TestBed.createComponent(TestCmp));
184+
const select = fixture.nativeElement.querySelector('select') as HTMLSelectElement;
185+
186+
act(() => {
187+
select.value = 'b';
188+
select.dispatchEvent(new Event('input'));
189+
});
190+
191+
expect(fixture.componentInstance.f().value()).toBe('b');
192+
expect(typeof fixture.componentInstance.f().value()).toBe('string');
193+
});
194+
});
195+
196+
function act<T>(fn: () => T): T {
197+
try {
198+
return fn();
199+
} finally {
200+
TestBed.tick();
201+
}
202+
}

0 commit comments

Comments
 (0)