Skip to content

Commit 702f2be

Browse files
edmundhungedmundhung
authored
fix(conform-dom): improve change detection with updateField (#1078)
1 parent 70475c0 commit 702f2be

File tree

3 files changed

+114
-28
lines changed

3 files changed

+114
-28
lines changed

‎.changeset/light-papayas-sing.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@conform-to/dom': patch
3+
'@conform-to/react': patch
4+
---
5+
6+
Fix change detection to avoid triggering unnecessary change events when a File input or select value hasn't actually changed

‎packages/conform-dom/dom.ts‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -486,17 +486,17 @@ export function normalizeStringValues(value: unknown): string[] | undefined {
486486
throw new Error('Expected string or string[] value for string based input');
487487
}
488488

489-
export function normalizeFileValues(value: unknown): FileList | undefined {
489+
export function normalizeFileValues(value: unknown): File[] | undefined {
490490
if (typeof value === 'undefined') return undefined;
491-
if (value === null) return createFileList([]);
491+
if (value === null) return [];
492492
if (isGlobalInstance(value, 'File'))
493-
return createFileList(value.name === '' && value.size === 0 ? [] : [value]);
494-
if (isGlobalInstance(value, 'FileList')) return value;
493+
return value.name === '' && value.size === 0 ? [] : [value];
494+
if (isGlobalInstance(value, 'FileList')) return Array.from(value);
495495
if (
496496
Array.isArray(value) &&
497497
value.every((item) => isGlobalInstance(item, 'File'))
498498
) {
499-
return createFileList(value);
499+
return value;
500500
}
501501

502502
throw new Error('Expected File, FileList or File[] for file input');
@@ -559,8 +559,14 @@ export function updateField(
559559
switch (element.type) {
560560
case 'file': {
561561
const files = normalizeFileValues(options.value);
562-
if (files && element.files !== files) {
563-
element.files = files;
562+
const currentFiles = Array.from(element.files ?? []);
563+
564+
if (
565+
files &&
566+
(files.length !== currentFiles.length ||
567+
files.some((file, i) => file !== currentFiles[i]))
568+
) {
569+
element.files = createFileList(files);
564570
isChanged = true;
565571
}
566572

@@ -646,7 +652,7 @@ export function updateField(
646652

647653
// If the select element is not multiple and the value is an empty array, unset the selected index
648654
// This is to prevent the select element from showing the first option as selected
649-
if (shouldUnselect) {
655+
if (shouldUnselect && element.selectedIndex !== -1) {
650656
element.selectedIndex = -1;
651657
isChanged = true;
652658
}

‎packages/conform-dom/tests/dom.browser.test.ts‎

Lines changed: 94 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,67 +16,107 @@ describe('updateField', () => {
1616
const input = document.createElement('input');
1717
input.type = 'text';
1818

19-
updateField(input, {
19+
const changed1 = updateField(input, {
2020
value: 'foo',
2121
defaultValue: 'foo',
2222
});
2323
expect(input.value).toBe('foo');
2424
expect(input.defaultValue).toBe('foo');
25+
expect(changed1).toBe(true);
2526

26-
updateField(input, {
27+
const changed2 = updateField(input, {
2728
value: null,
2829
});
2930

3031
expect(input.value).toBe('');
3132
expect(input.defaultValue).toBe('foo');
33+
expect(changed2).toBe(true);
34+
35+
const changed3 = updateField(input, {
36+
value: '',
37+
});
38+
expect(input.value).toBe('');
39+
expect(changed3).toBe(false);
40+
41+
const changed4 = updateField(input, {
42+
defaultValue: 'bar',
43+
});
44+
expect(input.value).toBe('');
45+
expect(input.defaultValue).toBe('bar');
46+
expect(changed4).toBe(false);
3247
});
3348

3449
it('supports checkbox', () => {
3550
const input = document.createElement('input');
3651
input.type = 'checkbox';
3752
input.checked = false;
3853

39-
updateField(input, {
54+
const changed1 = updateField(input, {
4055
value: 'on',
4156
});
4257

4358
expect(input.checked).toBe(true);
59+
expect(changed1).toBe(true);
4460

4561
input.value = 'test';
4662
input.checked = false;
47-
updateField(input, {
63+
const changed2 = updateField(input, {
4864
value: 'test',
4965
defaultValue: 'test',
5066
});
5167
expect(input.checked).toBe(true);
5268
expect(input.defaultChecked).toBe(true);
69+
expect(changed2).toBe(true);
5370

54-
updateField(input, { value: null });
71+
const changed3 = updateField(input, { value: null });
5572
expect(input.checked).toBe(false);
5673
expect(input.defaultChecked).toBe(true);
74+
expect(changed3).toBe(true);
75+
76+
const changed4 = updateField(input, { value: '' });
77+
expect(input.checked).toBe(false);
78+
expect(changed4).toBe(false);
79+
80+
const changed5 = updateField(input, { defaultValue: '' });
81+
expect(input.checked).toBe(false);
82+
expect(input.defaultChecked).toBe(false);
83+
expect(changed5).toBe(false);
5784
});
5885

5986
it('supports radio button', () => {
6087
const input = document.createElement('input');
6188
input.type = 'radio';
6289
input.checked = false;
6390

64-
updateField(input, { value: 'on' });
91+
const changed1 = updateField(input, { value: 'on' });
6592

6693
expect(input.checked).toBe(true);
94+
expect(changed1).toBe(true);
6795

6896
input.value = 'test';
6997
input.checked = false;
70-
updateField(input, {
98+
const changed2 = updateField(input, {
7199
value: 'test',
72100
defaultValue: 'test',
73101
});
74102
expect(input.checked).toBe(true);
75103
expect(input.defaultChecked).toBe(true);
104+
expect(changed2).toBe(true);
76105

77-
updateField(input, { value: null });
106+
// Unchecking a radio button doesn't report as changed
107+
const changed3 = updateField(input, { value: null });
78108
expect(input.checked).toBe(false);
79109
expect(input.defaultChecked).toBe(true);
110+
expect(changed3).toBe(false);
111+
112+
const changed4 = updateField(input, { value: '' });
113+
expect(input.checked).toBe(false);
114+
expect(changed4).toBe(false);
115+
116+
const changed5 = updateField(input, { defaultValue: '' });
117+
expect(input.checked).toBe(false);
118+
expect(input.defaultChecked).toBe(false);
119+
expect(changed5).toBe(false);
80120
});
81121

82122
it('supports file input', () => {
@@ -88,32 +128,43 @@ describe('updateField', () => {
88128
type: 'text/plain',
89129
});
90130

91-
updateField(input, { value: file });
131+
const changed1 = updateField(input, { value: file });
92132

93133
expect(input.files?.[0]).toEqual(file);
94134
expect(input.files?.length).toBe(1);
135+
expect(changed1).toBe(true);
95136

96137
input.multiple = true;
97138

98139
const file2 = new File(['SELECT * FROM users;'], 'example2.sql', {
99140
type: 'text/plain',
100141
});
101-
updateField(input, { value: [file, file2] });
142+
const changed2 = updateField(input, { value: [file, file2] });
102143

103144
expect(input.files?.[0]).toEqual(file);
104145
expect(input.files?.[1]).toEqual(file2);
105146
expect(input.files?.length).toBe(2);
147+
expect(changed2).toBe(true);
106148

107-
updateField(input, { value: null });
149+
const changed3 = updateField(input, { value: null });
108150
expect(input.files?.length).toBe(0);
151+
expect(changed3).toBe(true);
109152

110-
updateField(input, { value: createFileList([file, file2]) });
153+
const changed4 = updateField(input, {
154+
value: createFileList([file, file2]),
155+
});
111156
expect(input.files?.[0]).toEqual(file);
112157
expect(input.files?.[1]).toEqual(file2);
113158
expect(input.files?.length).toBe(2);
159+
expect(changed4).toBe(true);
160+
161+
const changed5 = updateField(input, { value: null });
162+
expect(input.files?.length).toBe(0);
163+
expect(changed5).toBe(true);
114164

115-
updateField(input, { value: emptyFile });
165+
const changed6 = updateField(input, { value: emptyFile });
116166
expect(input.files?.length).toBe(0);
167+
expect(changed6).toBe(false);
117168
});
118169

119170
it('supports select', () => {
@@ -127,20 +178,23 @@ describe('updateField', () => {
127178

128179
select.append(emptyOption, option1, option2);
129180

130-
updateField(select, { value: 'option2' });
181+
const changed1 = updateField(select, { value: 'option2' });
131182
expect(select.selectedIndex).toBe(2);
183+
expect(changed1).toBe(true);
132184

133-
updateField(select, { value: null });
185+
const changed2 = updateField(select, { value: null });
134186
expect(select.selectedIndex).toBe(-1);
187+
expect(changed2).toBe(true);
135188

136189
select.multiple = true;
137-
updateField(select, { value: ['option1', 'option2'] });
190+
const changed3 = updateField(select, { value: ['option1', 'option2'] });
138191
expect(select.options.item(0)?.selected).toBe(false);
139192
expect(select.options.item(1)?.selected).toBe(true);
140193
expect(select.options.item(2)?.selected).toBe(true);
141194
expect(select.options.length).toBe(3);
195+
expect(changed3).toBe(true);
142196

143-
updateField(select, {
197+
const changed4 = updateField(select, {
144198
value: ['option3', 'option1'],
145199
defaultValue: ['option3', 'option1'],
146200
});
@@ -153,8 +207,9 @@ describe('updateField', () => {
153207
expect(select.options.item(2)?.defaultSelected).toBe(false);
154208
expect(select.options.item(3)?.defaultSelected).toBe(true);
155209
expect(select.options.length).toBe(4);
210+
expect(changed4).toBe(true);
156211

157-
updateField(select, { value: null });
212+
const changed5 = updateField(select, { value: null });
158213
expect(select.options.item(0)?.selected).toBe(false);
159214
expect(select.options.item(1)?.selected).toBe(false);
160215
expect(select.options.item(2)?.selected).toBe(false);
@@ -163,21 +218,40 @@ describe('updateField', () => {
163218
expect(select.options.item(1)?.defaultSelected).toBe(true);
164219
expect(select.options.item(2)?.defaultSelected).toBe(false);
165220
expect(select.options.item(3)?.defaultSelected).toBe(true);
221+
expect(changed5).toBe(true);
222+
223+
const changed6 = updateField(select, { value: [] });
224+
expect(select.options.item(0)?.selected).toBe(false);
225+
expect(select.options.item(1)?.selected).toBe(false);
226+
expect(select.options.item(2)?.selected).toBe(false);
227+
expect(select.options.item(3)?.selected).toBe(false);
228+
expect(changed6).toBe(false);
166229
});
167230

168231
it('supports textarea', () => {
169232
const textarea = document.createElement('textarea');
170233

171-
updateField(textarea, {
234+
const changed1 = updateField(textarea, {
172235
value: 'hello world',
173236
defaultValue: 'hello world',
174237
});
175238
expect(textarea.value).toBe('hello world');
176239
expect(textarea.defaultValue).toBe('hello world');
240+
expect(changed1).toBe(true);
177241

178-
updateField(textarea, { value: null });
242+
const changed2 = updateField(textarea, { value: null });
179243
expect(textarea.value).toBe('');
180244
expect(textarea.defaultValue).toBe('hello world');
245+
expect(changed2).toBe(true);
246+
247+
const changed3 = updateField(textarea, { value: '' });
248+
expect(textarea.value).toBe('');
249+
expect(changed3).toBe(false);
250+
251+
const changed4 = updateField(textarea, { defaultValue: 'foo' });
252+
expect(textarea.value).toBe('');
253+
expect(textarea.defaultValue).toBe('foo');
254+
expect(changed4).toBe(false);
181255
});
182256
});
183257

0 commit comments

Comments
 (0)