diff --git a/.changeset/honest-shoes-jam.md b/.changeset/honest-shoes-jam.md new file mode 100644 index 000000000000..dd217235e7d4 --- /dev/null +++ b/.changeset/honest-shoes-jam.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fix: use `$derived` for form fields diff --git a/packages/kit/src/runtime/app/server/remote/form.js b/packages/kit/src/runtime/app/server/remote/form.js index ba6562bf3423..937a0cc44a92 100644 --- a/packages/kit/src/runtime/app/server/remote/form.js +++ b/packages/kit/src/runtime/app/server/remote/form.js @@ -207,6 +207,7 @@ export function form(validate_or_fn, maybe_fn) { return create_field_proxy( {}, () => data?.input ?? {}, + () => {}, (path, value) => { if (data) { // don't override a submission diff --git a/packages/kit/src/runtime/client/remote-functions/form.svelte.js b/packages/kit/src/runtime/client/remote-functions/form.svelte.js index 133f7351abde..0f85fe5801ee 100644 --- a/packages/kit/src/runtime/client/remote-functions/form.svelte.js +++ b/packages/kit/src/runtime/client/remote-functions/form.svelte.js @@ -16,7 +16,9 @@ import { create_field_proxy, deep_set, set_nested_value, - throw_on_old_property_access + throw_on_old_property_access, + split_path, + build_path_string } from '../../form-utils.svelte.js'; /** @@ -62,6 +64,13 @@ export function form(id) { */ let input = $state.raw({}); + // TODO 3.0: Remove versions state and related logic; it's a workaround for $derived not updating when created inside $effects + /** + * This allows us to update individual fields granularly + * @type {Record} + */ + const versions = $state({}); + /** @type {Record} */ let issues = $state.raw({}); @@ -200,6 +209,13 @@ export function form(id) { } else { input = {}; + for (const [key, value] of Object.entries(versions)) { + if (value !== undefined) { + versions[key] ??= 0; + versions[key] += 1; + } + } + if (form_result.refreshes) { refresh_queries(form_result.refreshes, updates); } else { @@ -378,6 +394,18 @@ export function form(id) { element.type === 'checkbox' && !element.checked ? null : element.value ); } + + versions[name] ??= 0; + versions[name] += 1; + + const path = split_path(name); + + while (path.pop() !== undefined) { + const name = build_path_string(path); + + versions[name] ??= 0; + versions[name] += 1; + } }); return () => { @@ -470,6 +498,7 @@ export function form(id) { create_field_proxy( {}, () => input, + (path) => versions[path], (path, value) => { if (path.length === 0) { input = value; diff --git a/packages/kit/src/runtime/form-utils.svelte.js b/packages/kit/src/runtime/form-utils.svelte.js index fef4689a0dd6..c0e2f3f4d9db 100644 --- a/packages/kit/src/runtime/form-utils.svelte.js +++ b/packages/kit/src/runtime/form-utils.svelte.js @@ -3,6 +3,7 @@ /** @import { StandardSchemaV1 } from '@standard-schema/spec' */ import { DEV } from 'esm-env'; +import { untrack } from 'svelte'; /** * Sets a value in a nested object using a path string, not mutating the original object but returning a new object @@ -170,19 +171,27 @@ export function deep_get(object, path) { * Creates a proxy-based field accessor for form data * @param {any} target - Function or empty POJO * @param {() => Record} get_input - Function to get current input data + * @param {(path: string) => void} depend - Function to make an effect depend on a specific field * @param {(path: (string | number)[], value: any) => void} set_input - Function to set input data * @param {() => Record} get_issues - Function to get current issues * @param {(string | number)[]} path - Current access path * @returns {any} Proxy object with name(), value(), and issues() methods */ -export function create_field_proxy(target, get_input, set_input, get_issues, path = []) { +export function create_field_proxy(target, get_input, depend, set_input, get_issues, path = []) { + const path_string = build_path_string(path); + + const get_value = () => { + depend(path_string); + return untrack(() => deep_get(get_input(), path)); + }; + return new Proxy(target, { get(target, prop) { if (typeof prop === 'symbol') return target[prop]; // Handle array access like jobs[0] if (/^\d+$/.test(prop)) { - return create_field_proxy({}, get_input, set_input, get_issues, [ + return create_field_proxy({}, get_input, depend, set_input, get_issues, [ ...path, parseInt(prop, 10) ]); @@ -195,18 +204,17 @@ export function create_field_proxy(target, get_input, set_input, get_issues, pat set_input(path, newValue); return newValue; }; - return create_field_proxy(set_func, get_input, set_input, get_issues, [...path, prop]); + return create_field_proxy(set_func, get_input, depend, set_input, get_issues, [ + ...path, + prop + ]); } if (prop === 'value') { - const value_func = function () { - // TODO Ideally we'd create a $derived just above and use it here but we can't because of push_reaction which prevents - // changes to deriveds created within an effect to rerun the effect - an argument for - // reverting that change in async mode? - // TODO we did that in Svelte now; bump Svelte version and use $derived here - return deep_get(get_input(), path); - }; - return create_field_proxy(value_func, get_input, set_input, get_issues, [...path, prop]); + return create_field_proxy(get_value, get_input, depend, set_input, get_issues, [ + ...path, + prop + ]); } if (prop === 'issues' || prop === 'allIssues') { @@ -226,7 +234,10 @@ export function create_field_proxy(target, get_input, set_input, get_issues, pat })); }; - return create_field_proxy(issues_func, get_input, set_input, get_issues, [...path, prop]); + return create_field_proxy(issues_func, get_input, depend, set_input, get_issues, [ + ...path, + prop + ]); } if (prop === 'as') { @@ -269,8 +280,7 @@ export function create_field_proxy(target, get_input, set_input, get_issues, pat value: { enumerable: true, get() { - const input = get_input(); - return deep_get(input, path); + return get_value(); } } }); @@ -293,8 +303,7 @@ export function create_field_proxy(target, get_input, set_input, get_issues, pat checked: { enumerable: true, get() { - const input = get_input(); - const value = deep_get(input, path); + const value = get_value(); if (type === 'radio') { return value === input_value; @@ -317,8 +326,7 @@ export function create_field_proxy(target, get_input, set_input, get_issues, pat files: { enumerable: true, get() { - const input = get_input(); - const value = deep_get(input, path); + const value = get_value(); // Convert File/File[] to FileList-like object if (value instanceof File) { @@ -358,20 +366,21 @@ export function create_field_proxy(target, get_input, set_input, get_issues, pat value: { enumerable: true, get() { - const input = get_input(); - const value = deep_get(input, path); - + const value = get_value(); return value != null ? String(value) : ''; } } }); }; - return create_field_proxy(as_func, get_input, set_input, get_issues, [...path, 'as']); + return create_field_proxy(as_func, get_input, depend, set_input, get_issues, [ + ...path, + 'as' + ]); } // Handle property access (nested fields) - return create_field_proxy({}, get_input, set_input, get_issues, [...path, prop]); + return create_field_proxy({}, get_input, depend, set_input, get_issues, [...path, prop]); } }); } @@ -381,7 +390,7 @@ export function create_field_proxy(target, get_input, set_input, get_issues, pat * @param {(string | number)[]} path * @returns {string} */ -function build_path_string(path) { +export function build_path_string(path) { let result = ''; for (const segment of path) { diff --git a/packages/kit/test/apps/basics/src/routes/remote/form/select-untouched/+page.svelte b/packages/kit/test/apps/basics/src/routes/remote/form/select-untouched/+page.svelte new file mode 100644 index 000000000000..5caecdde39f2 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/remote/form/select-untouched/+page.svelte @@ -0,0 +1,15 @@ + + +
+ + + + + +
diff --git a/packages/kit/test/apps/basics/src/routes/remote/form/select-untouched/form.remote.ts b/packages/kit/test/apps/basics/src/routes/remote/form/select-untouched/form.remote.ts new file mode 100644 index 000000000000..7e6e18b25e37 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/remote/form/select-untouched/form.remote.ts @@ -0,0 +1,10 @@ +import { form } from '$app/server'; +import * as v from 'valibot'; + +export const myform = form( + v.object({ + message: v.string(), + number: v.picklist(['one', 'two', 'three']) + }), + (_data) => {} +); diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index bf408ea7440b..f9d3e86b4adf 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1968,6 +1968,18 @@ test.describe('remote functions', () => { const arrayValue = await page.locator('#array-value').textContent(); expect(JSON.parse(arrayValue)).toEqual([{ leaf: 'array-0-leaf' }, { leaf: 'array-1-leaf' }]); }); + + test('selects are not nuked when unrelated controls change', async ({ + page, + javaScriptEnabled + }) => { + if (!javaScriptEnabled) return; + + await page.goto('/remote/form/select-untouched'); + + await page.fill('input', 'hello'); + await expect(page.locator('select')).toHaveValue('one'); + }); }); test.describe('params prop', () => {