Skip to content

Commit 874450a

Browse files
fix: granular updates of field.value() (#14621)
* fix: use `$derived` for form fields * make it work * const * Update packages/kit/src/runtime/client/remote-functions/form.svelte.js Co-authored-by: Simon H <[email protected]> --------- Co-authored-by: Simon H <[email protected]>
1 parent d5cfb22 commit 874450a

File tree

7 files changed

+106
-25
lines changed

7 files changed

+106
-25
lines changed

.changeset/honest-shoes-jam.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@sveltejs/kit': patch
3+
---
4+
5+
fix: use `$derived` for form fields

packages/kit/src/runtime/app/server/remote/form.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,7 @@ export function form(validate_or_fn, maybe_fn) {
209209
return create_field_proxy(
210210
{},
211211
() => data?.input ?? {},
212+
() => {},
212213
(path, value) => {
213214
if (data) {
214215
// don't override a submission

packages/kit/src/runtime/client/remote-functions/form.svelte.js

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@ import {
1616
create_field_proxy,
1717
deep_set,
1818
set_nested_value,
19-
throw_on_old_property_access
19+
throw_on_old_property_access,
20+
split_path,
21+
build_path_string
2022
} from '../../form-utils.svelte.js';
2123

2224
/**
@@ -62,6 +64,13 @@ export function form(id) {
6264
*/
6365
let input = $state.raw({});
6466

67+
// TODO 3.0: Remove versions state and related logic; it's a workaround for $derived not updating when created inside $effects
68+
/**
69+
* This allows us to update individual fields granularly
70+
* @type {Record<string, number>}
71+
*/
72+
const versions = $state({});
73+
6574
/** @type {Record<string, InternalRemoteFormIssue[]>} */
6675
let issues = $state.raw({});
6776

@@ -212,6 +221,13 @@ export function form(id) {
212221
} else {
213222
input = {};
214223

224+
for (const [key, value] of Object.entries(versions)) {
225+
if (value !== undefined) {
226+
versions[key] ??= 0;
227+
versions[key] += 1;
228+
}
229+
}
230+
215231
if (form_result.refreshes) {
216232
refresh_queries(form_result.refreshes, updates);
217233
} else {
@@ -390,6 +406,18 @@ export function form(id) {
390406
element.type === 'checkbox' && !element.checked ? null : element.value
391407
);
392408
}
409+
410+
versions[name] ??= 0;
411+
versions[name] += 1;
412+
413+
const path = split_path(name);
414+
415+
while (path.pop() !== undefined) {
416+
const name = build_path_string(path);
417+
418+
versions[name] ??= 0;
419+
versions[name] += 1;
420+
}
393421
});
394422

395423
return () => {
@@ -482,6 +510,7 @@ export function form(id) {
482510
create_field_proxy(
483511
{},
484512
() => input,
513+
(path) => versions[path],
485514
(path, value) => {
486515
if (path.length === 0) {
487516
input = value;

packages/kit/src/runtime/form-utils.svelte.js

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
/** @import { StandardSchemaV1 } from '@standard-schema/spec' */
44

55
import { DEV } from 'esm-env';
6+
import { untrack } from 'svelte';
67

78
/**
89
* Sets a value in a nested object using a path string, not mutating the original object but returning a new object
@@ -174,19 +175,27 @@ export function deep_get(object, path) {
174175
* Creates a proxy-based field accessor for form data
175176
* @param {any} target - Function or empty POJO
176177
* @param {() => Record<string, any>} get_input - Function to get current input data
178+
* @param {(path: string) => void} depend - Function to make an effect depend on a specific field
177179
* @param {(path: (string | number)[], value: any) => void} set_input - Function to set input data
178180
* @param {() => Record<string, InternalRemoteFormIssue[]>} get_issues - Function to get current issues
179181
* @param {(string | number)[]} path - Current access path
180182
* @returns {any} Proxy object with name(), value(), and issues() methods
181183
*/
182-
export function create_field_proxy(target, get_input, set_input, get_issues, path = []) {
184+
export function create_field_proxy(target, get_input, depend, set_input, get_issues, path = []) {
185+
const path_string = build_path_string(path);
186+
187+
const get_value = () => {
188+
depend(path_string);
189+
return untrack(() => deep_get(get_input(), path));
190+
};
191+
183192
return new Proxy(target, {
184193
get(target, prop) {
185194
if (typeof prop === 'symbol') return target[prop];
186195

187196
// Handle array access like jobs[0]
188197
if (/^\d+$/.test(prop)) {
189-
return create_field_proxy({}, get_input, set_input, get_issues, [
198+
return create_field_proxy({}, get_input, depend, set_input, get_issues, [
190199
...path,
191200
parseInt(prop, 10)
192201
]);
@@ -199,18 +208,17 @@ export function create_field_proxy(target, get_input, set_input, get_issues, pat
199208
set_input(path, newValue);
200209
return newValue;
201210
};
202-
return create_field_proxy(set_func, get_input, set_input, get_issues, [...path, prop]);
211+
return create_field_proxy(set_func, get_input, depend, set_input, get_issues, [
212+
...path,
213+
prop
214+
]);
203215
}
204216

205217
if (prop === 'value') {
206-
const value_func = function () {
207-
// TODO Ideally we'd create a $derived just above and use it here but we can't because of push_reaction which prevents
208-
// changes to deriveds created within an effect to rerun the effect - an argument for
209-
// reverting that change in async mode?
210-
// TODO we did that in Svelte now; bump Svelte version and use $derived here
211-
return deep_get(get_input(), path);
212-
};
213-
return create_field_proxy(value_func, get_input, set_input, get_issues, [...path, prop]);
218+
return create_field_proxy(get_value, get_input, depend, set_input, get_issues, [
219+
...path,
220+
prop
221+
]);
214222
}
215223

216224
if (prop === 'issues' || prop === 'allIssues') {
@@ -230,7 +238,10 @@ export function create_field_proxy(target, get_input, set_input, get_issues, pat
230238
}));
231239
};
232240

233-
return create_field_proxy(issues_func, get_input, set_input, get_issues, [...path, prop]);
241+
return create_field_proxy(issues_func, get_input, depend, set_input, get_issues, [
242+
...path,
243+
prop
244+
]);
234245
}
235246

236247
if (prop === 'as') {
@@ -273,8 +284,7 @@ export function create_field_proxy(target, get_input, set_input, get_issues, pat
273284
value: {
274285
enumerable: true,
275286
get() {
276-
const input = get_input();
277-
return deep_get(input, path);
287+
return get_value();
278288
}
279289
}
280290
});
@@ -297,8 +307,7 @@ export function create_field_proxy(target, get_input, set_input, get_issues, pat
297307
checked: {
298308
enumerable: true,
299309
get() {
300-
const input = get_input();
301-
const value = deep_get(input, path);
310+
const value = get_value();
302311

303312
if (type === 'radio') {
304313
return value === input_value;
@@ -321,8 +330,7 @@ export function create_field_proxy(target, get_input, set_input, get_issues, pat
321330
files: {
322331
enumerable: true,
323332
get() {
324-
const input = get_input();
325-
const value = deep_get(input, path);
333+
const value = get_value();
326334

327335
// Convert File/File[] to FileList-like object
328336
if (value instanceof File) {
@@ -362,20 +370,21 @@ export function create_field_proxy(target, get_input, set_input, get_issues, pat
362370
value: {
363371
enumerable: true,
364372
get() {
365-
const input = get_input();
366-
const value = deep_get(input, path);
367-
373+
const value = get_value();
368374
return value != null ? String(value) : '';
369375
}
370376
}
371377
});
372378
};
373379

374-
return create_field_proxy(as_func, get_input, set_input, get_issues, [...path, 'as']);
380+
return create_field_proxy(as_func, get_input, depend, set_input, get_issues, [
381+
...path,
382+
'as'
383+
]);
375384
}
376385

377386
// Handle property access (nested fields)
378-
return create_field_proxy({}, get_input, set_input, get_issues, [...path, prop]);
387+
return create_field_proxy({}, get_input, depend, set_input, get_issues, [...path, prop]);
379388
}
380389
});
381390
}
@@ -385,7 +394,7 @@ export function create_field_proxy(target, get_input, set_input, get_issues, pat
385394
* @param {(string | number)[]} path
386395
* @returns {string}
387396
*/
388-
function build_path_string(path) {
397+
export function build_path_string(path) {
389398
let result = '';
390399

391400
for (const segment of path) {
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<script>
2+
import { myform } from './form.remote.js';
3+
</script>
4+
5+
<form {...myform}>
6+
<input {...myform.fields.message.as('text')} />
7+
8+
<select {...myform.fields.number.as('select')}>
9+
<option>one</option>
10+
<option>two</option>
11+
<option>three</option>
12+
</select>
13+
14+
<button>submit</button>
15+
</form>
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import { form } from '$app/server';
2+
import * as v from 'valibot';
3+
4+
export const myform = form(
5+
v.object({
6+
message: v.string(),
7+
number: v.picklist(['one', 'two', 'three'])
8+
}),
9+
(_data) => {}
10+
);

packages/kit/test/apps/basics/test/test.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1977,6 +1977,18 @@ test.describe('remote functions', () => {
19771977
const arrayValue = await page.locator('#array-value').textContent();
19781978
expect(JSON.parse(arrayValue)).toEqual([{ leaf: 'array-0-leaf' }, { leaf: 'array-1-leaf' }]);
19791979
});
1980+
1981+
test('selects are not nuked when unrelated controls change', async ({
1982+
page,
1983+
javaScriptEnabled
1984+
}) => {
1985+
if (!javaScriptEnabled) return;
1986+
1987+
await page.goto('/remote/form/select-untouched');
1988+
1989+
await page.fill('input', 'hello');
1990+
await expect(page.locator('select')).toHaveValue('one');
1991+
});
19801992
});
19811993

19821994
test.describe('params prop', () => {

0 commit comments

Comments
 (0)