Skip to content

Commit 6bfdd4b

Browse files
committed
fixup! fix(forms): prevent date inputs from being marked dirty on init
1 parent 5726517 commit 6bfdd4b

2 files changed

Lines changed: 118 additions & 51 deletions

File tree

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

Lines changed: 34 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -32,32 +32,6 @@ import {
3232
} from './native';
3333
import {observeSelectMutations} from './select';
3434

35-
/**
36-
* Normalizes a control value for comparison, collapsing an invalid `Date` to `null`.
37-
*
38-
* A `Date` with a `NaN` timestamp has no representation a date-like input can hold: assigning it to
39-
* `valueAsDate` empties the input, which then reads back as `null`. Treating the two as the same
40-
* value keeps that round-trip from looking like an edit.
41-
*/
42-
function normalizeControlValue(value: unknown): unknown {
43-
return value instanceof Date && Number.isNaN(value.getTime()) ? null : value;
44-
}
45-
46-
/**
47-
* Compares two control values, treating `Date`s for the same instant as equal.
48-
*
49-
* Date-like inputs are read through `valueAsDate`, which returns a fresh `Date` on every access, so
50-
* identity comparison alone would report a change even when the input was never touched.
51-
*/
52-
function controlValuesEqual(a: unknown, b: unknown): boolean {
53-
const normalizedA = normalizeControlValue(a);
54-
const normalizedB = normalizeControlValue(b);
55-
if (normalizedA instanceof Date && normalizedB instanceof Date) {
56-
return normalizedA.getTime() === normalizedB.getTime();
57-
}
58-
return Object.is(normalizedA, normalizedB);
59-
}
60-
6135
export function nativeControlCreate(
6236
host: ControlDirectiveHost,
6337
parent: FormField<unknown>,
@@ -67,40 +41,42 @@ export function nativeControlCreate(
6741
validityMonitor: InputValidityMonitor,
6842
): () => void {
6943
let updateMode = false;
70-
// While true, a write that leaves the value untouched must not dirty the field.
71-
//
72-
// A native validity change isn't necessarily a user edit: the browser runs the `:valid` /
73-
// `:invalid` animation as soon as the input is rendered, which would otherwise dirty every
74-
// date-like field on load (#69632). Syncs that clear a parse error are exempt, because a parse
75-
// error can only exist in response to the user typing something the input couldn't parse.
76-
let dirtyOnlyIfValueChanged = false;
7744
const input = parent.nativeFormElement;
45+
// The input's value as the DOM itself holds it, recorded after each write we make.
46+
//
47+
// Comparing model values can't tell a user edit from the browser reshaping what we wrote: an
48+
// unparseable string or a `Date` the input can't represent is rejected and leaves the value
49+
// empty, and a `Date` carrying more precision than the input keeps is truncated. In each case the
50+
// value read back differs from the model through no action of the user, which would dirty every
51+
// date-like field on load (#69632). The DOM's own string is the one representation both sides
52+
// agree on.
53+
let lastWrittenDomValue = input.value;
7854

7955
// TODO: (perf) ok to always create this?
8056
const parser = createParser(
8157
// Read from the model value
8258
() => parent.state().value(),
8359
// Write to the buffered "control value"
84-
(rawValue: unknown) => {
85-
const state = parent.state();
86-
if (dirtyOnlyIfValueChanged && controlValuesEqual(untracked(state.controlValue), rawValue)) {
87-
return;
88-
}
89-
// Outside of `dirtyOnlyIfValueChanged` we intentionally write even when the value is
90-
// unchanged, so that re-entering the same value still dirties the field.
91-
state.controlValue.set(rawValue);
92-
},
60+
(rawValue: unknown) => parent.state().controlValue.set(rawValue),
9361
// Our parse function doesn't care about the raw value that gets passed in,
9462
// It just reads the newly parsed value directly off the input element.
9563
(_rawValue: unknown) => getNativeControlValue(input, parent.state().value, validityMonitor),
9664
);
9765

9866
parseErrorsSource.set(parser.errors);
67+
// Writes to the DOM and records what the DOM ended up holding, which is not necessarily what we
68+
// asked for: the browser rejects values it can't parse and truncates ones carrying more precision
69+
// than the input type keeps.
70+
const writeNativeControlValue = (value: unknown) => {
71+
setNativeControlValue(input, value);
72+
lastWrittenDomValue = input.value;
73+
};
74+
9975
parent.onReset = () => {
10076
parser.reset();
10177
const value = parent.state().value();
10278
bindings['controlValue'] = value;
103-
setNativeControlValue(input, value);
79+
writeNativeControlValue(value);
10480
};
10581
// Pass undefined as the raw value since the parse function doesn't care about it.
10682
host.listenToDom('input', () => {
@@ -115,14 +91,21 @@ export function nativeControlCreate(
11591
// TODO: move extraction to first update pass?
11692
if (isInput(input) && inputRequiresValidityTracking(input)) {
11793
validityMonitor.watchValidity(parent.destroyRef, input, () => {
118-
// Resolving a parse error is always a user edit, so let those syncs dirty the field even
119-
// when the parsed value is unchanged.
120-
dirtyOnlyIfValueChanged = untracked(parser.errors).length === 0;
121-
try {
122-
parser.setRawValue(undefined);
123-
} finally {
124-
dirtyOnlyIfValueChanged = false;
94+
// The browser runs the `:valid` / `:invalid` animation as soon as the input renders, which is
95+
// not a user edit. If the DOM still holds exactly what we last wrote then nothing changed and
96+
// there is nothing to sync.
97+
//
98+
// Two states are exempt, because in both the user did edit the input while its `value` stayed
99+
// empty: entering text the input can't convert (`badInput`, which reports an empty `value`),
100+
// and clearing that text again, which resolves the parse error the previous sync recorded.
101+
if (
102+
untracked(parser.errors).length === 0 &&
103+
!validityMonitor.isBadInput(input) &&
104+
input.value === lastWrittenDomValue
105+
) {
106+
return;
125107
}
108+
parser.setRawValue(undefined);
126109
});
127110
}
128111

@@ -179,7 +162,7 @@ export function nativeControlCreate(
179162
input.type === 'radio' && bindingUpdated(bindings, 'radioValue', input.value);
180163

181164
if (controlValueChanged || radioValueChanged) {
182-
setNativeControlValue(input, controlValue);
165+
writeNativeControlValue(controlValue);
183166
}
184167

185168
updateMode = true;

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

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5690,6 +5690,90 @@ describe('field directive', () => {
56905690
expect(Number.isNaN((field().value() as Date).getTime())).toBe(true);
56915691
});
56925692

5693+
// A string the input can't parse as a date is rejected outright, so the DOM value stays empty
5694+
// while the model still holds the original string. Comparing the two as values reports a change
5695+
// on every sync and dirties the field on load. See #69632.
5696+
it('should not be dirty when the initial validity animation fires on an unparseable string model', () => {
5697+
@Component({
5698+
imports: [FormField],
5699+
template: `<input type="date" [formField]="f" />`,
5700+
})
5701+
class TestCmp {
5702+
f = form(signal('invalid-date'));
5703+
}
5704+
5705+
const fix = act(() => TestBed.createComponent(TestCmp));
5706+
const input = fix.nativeElement.firstChild as HTMLInputElement;
5707+
const field = fix.componentInstance.f;
5708+
5709+
// The browser refuses a value it can't parse as a date, leaving the input empty.
5710+
expect(input.value).toBe('');
5711+
expect(field().dirty()).toBe(false);
5712+
5713+
act(() => {
5714+
input.dispatchEvent(
5715+
new AnimationEvent('animationstart', {animationName: 'ng-invalid', bubbles: true}),
5716+
);
5717+
});
5718+
5719+
expect(field().dirty()).toBe(false);
5720+
// The model keeps the value the app supplied; the sync must not silently empty it.
5721+
expect(field().value()).toBe('invalid-date');
5722+
});
5723+
5724+
// A `Date` can hold more precision than any of these inputs keeps: `date` drops the time,
5725+
// `month` the day, `time` the date, and `week` snaps to the start of the week. The value read
5726+
// back therefore differs from the model through no action of the user. See #69632.
5727+
it('should not be dirty when the initial validity animation fires on a Date the input truncates', () => {
5728+
@Component({
5729+
imports: [FormField],
5730+
template: `
5731+
<input type="date" [formField]="f.date" />
5732+
<input type="month" [formField]="f.month" />
5733+
<input type="time" [formField]="f.time" />
5734+
<input type="week" [formField]="f.week" />
5735+
`,
5736+
})
5737+
class TestCmp {
5738+
f = form(
5739+
signal<{date: Date | null; month: Date | null; time: Date | null; week: Date | null}>({
5740+
date: new Date('2024-03-15T12:34:56.000Z'),
5741+
month: new Date('2024-03-15T12:34:56.000Z'),
5742+
time: new Date('2024-03-15T12:34:56.000Z'),
5743+
week: new Date('2024-03-15T12:34:56.000Z'),
5744+
}),
5745+
);
5746+
}
5747+
5748+
const fix = act(() => TestBed.createComponent(TestCmp));
5749+
const cmp = fix.componentInstance as TestCmp;
5750+
const inputs = Array.from(
5751+
(fix.nativeElement as HTMLElement).querySelectorAll('input'),
5752+
) as HTMLInputElement[];
5753+
const fields = [cmp.f.date, cmp.f.month, cmp.f.time, cmp.f.week];
5754+
5755+
expect(inputs.map((i) => i.getAttribute('type'))).toEqual(['date', 'month', 'time', 'week']);
5756+
expect(fields.map((field) => field().dirty())).toEqual([false, false, false, false]);
5757+
5758+
act(() => {
5759+
for (const input of inputs) {
5760+
input.dispatchEvent(
5761+
new AnimationEvent('animationstart', {animationName: 'ng-invalid', bubbles: true}),
5762+
);
5763+
}
5764+
});
5765+
5766+
expect(fields.map((field) => field().dirty())).toEqual([false, false, false, false]);
5767+
// Each model keeps its full instant rather than the truncated value the input displays.
5768+
const instant = new Date('2024-03-15T12:34:56.000Z').getTime();
5769+
expect(fields.map((field) => (field().value() as Date).getTime())).toEqual([
5770+
instant,
5771+
instant,
5772+
instant,
5773+
instant,
5774+
]);
5775+
});
5776+
56935777
// The initial `:valid` / `:invalid` animation fires for every input type we track validity on,
56945778
// not just `date`, so none of them may dirty their field on load. See #69632.
56955779
//

0 commit comments

Comments
 (0)