Preserve server issues when doing preflight-only validation

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

@@ -5,11 +5,12 @@ import { get_request_store } from '@sveltejs/kit/internal/server';
 import { DEV } from 'esm-env';
 import {
 	convert_formdata,
-	flatten_issues,
 	create_field_proxy,
 	set_nested_value,
 	throw_on_old_property_access,
-	deep_set
+	deep_set,
+	normalize_issue,
+	flatten_issues
 } from '../../../form-utils.svelte.js';
 import { get_cache, run_remote_function } from './shared.js';
 
@@ -142,7 +143,7 @@ export function form(validate_or_fn, maybe_fn) {
 					}
 				}
 
-				/** @type {{ submission: true, input?: Record<string, any>, issues?: Record<string, InternalRemoteFormIssue[]>, result: Output }} */
+				/** @type {{ submission: true, input?: Record<string, any>, issues?: InternalRemoteFormIssue[], result: Output }} */
 				const output = {};
 
 				// make it possible to differentiate between user submission and programmatic `field.set(...)` updates
@@ -209,6 +210,8 @@ export function form(validate_or_fn, maybe_fn) {
 		Object.defineProperty(instance, 'fields', {
 			get() {
 				const data = get_cache(__)?.[''];
+				const issues = flatten_issues(data?.issues ?? []);
+
 				return create_field_proxy(
 					{},
 					() => data?.input ?? {},
@@ -224,7 +227,7 @@ export function form(validate_or_fn, maybe_fn) {
 
 						(get_cache(__)[''] ??= {}).input = input;
 					},
-					() => data?.issues ?? {}
+					() => issues
 				);
 			}
 		});
@@ -293,13 +296,13 @@ export function form(validate_or_fn, maybe_fn) {
 }
 
 /**
- * @param {{ issues?: Record<string, any>, input?: Record<string, any>, result: any }} output
+ * @param {{ issues?: InternalRemoteFormIssue[], input?: Record<string, any>, result: any }} output
  * @param {readonly StandardSchemaV1.Issue[]} issues
  * @param {boolean} is_remote_request
  * @param {FormData} form_data
  */
 function handle_issues(output, issues, is_remote_request, form_data) {
-	output.issues = flatten_issues(issues);
+	output.issues = issues.map((issue) => normalize_issue(issue, true));
 
 	// if it was a progressively-enhanced submission, we don't need
 	// to return the input — it's already there

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

@@ -18,27 +18,29 @@ import {
 	set_nested_value,
 	throw_on_old_property_access,
 	split_path,
-	build_path_string
+	build_path_string,
+	normalize_issue
 } from '../../form-utils.svelte.js';
 
 /**
- * Merge client issues into server issues
- * @param {Record<string, InternalRemoteFormIssue[]>} current_issues
- * @param {Record<string, InternalRemoteFormIssue[]>} client_issues
- * @returns {Record<string, InternalRemoteFormIssue[]>}
+ * Merge client issues into server issues. Server issues are persisted unless
+ * a client-issue exists for the same path, in which case the client-issue overrides it.
+ * @param {FormData} form_data
+ * @param {InternalRemoteFormIssue[]} current_issues
+ * @param {InternalRemoteFormIssue[]} client_issues
+ * @returns {InternalRemoteFormIssue[]}
  */
-function merge_with_server_issues(current_issues, client_issues) {
-	const merged_issues = Object.fromEntries(
-		Object.entries(current_issues)
-			.map(([key, issue_list]) => [key, issue_list.filter((issue) => issue.server)])
-			.filter(([, issue_list]) => issue_list.length > 0)
-	);
-
-	for (const [key, new_issue_list] of Object.entries(client_issues)) {
-		merged_issues[key] = [...(merged_issues[key] || []), ...new_issue_list];
-	}
+function merge_with_server_issues(form_data, current_issues, client_issues) {
+	const merged = [
+		...current_issues.filter(
+			(issue) => issue.server && !client_issues.some((i) => i.name === issue.name)
+		),
+		...client_issues
+	];
 
-	return merged_issues;
+	const keys = Array.from(form_data.keys());
+
+	return merged.sort((a, b) => keys.indexOf(a.name) - keys.indexOf(b.name));
 }
 
 /**
@@ -77,8 +79,10 @@ export function form(id) {
 		 */
 		const version_reads = new Set();
 
-		/** @type {Record<string, InternalRemoteFormIssue[]>} */
-		let issues = $state.raw({});
+		/** @type {InternalRemoteFormIssue[]} */
+		let raw_issues = $state.raw([]);
+
+		const issues = $derived(flatten_issues(raw_issues));
 
 		/** @type {any} */
 		let result = $state.raw(remote_responses[action_id]);
@@ -132,8 +136,11 @@ export function form(id) {
 			const validated = await preflight_schema?.['~standard'].validate(data);
 
 			if (validated?.issues) {
-				const client_issues = flatten_issues(validated.issues, false);
-				issues = merge_with_server_issues(issues, client_issues);
+				raw_issues = merge_with_server_issues(
+					form_data,
+					raw_issues,
+					validated.issues.map((issue) => normalize_issue(issue, false))
+				);
 				return;
 			}
 
@@ -223,14 +230,7 @@ export function form(id) {
 					const form_result = /** @type { RemoteFunctionResponse} */ (await response.json());
 
 					if (form_result.type === 'result') {
-						({ issues = {}, result } = devalue.parse(form_result.result, app.decoders));
-
-						// Mark server issues with server: true
-						for (const issue_list of Object.values(issues)) {
-							for (const issue of issue_list) {
-								issue.server = true;
-							}
-						}
+						({ issues: raw_issues = [], result } = devalue.parse(form_result.result, app.decoders));
 
 						if (issues.$) {
 							release_overrides(updates);
@@ -582,7 +582,7 @@ export function form(id) {
 
 					const form_data = new FormData(element, submitter);
 
-					/** @type {readonly StandardSchemaV1.Issue[]} */
+					/** @type {InternalRemoteFormIssue[]} */
 					let array = [];
 
 					const validated = await preflight_schema?.['~standard'].validate(convert(form_data));
@@ -592,7 +592,7 @@ export function form(id) {
 					}
 
 					if (validated?.issues) {
-						array = validated.issues;
+						array = validated.issues.map((issue) => normalize_issue(issue, false));
 					} else if (!preflightOnly) {
 						form_data.set('sveltekit:validate_only', 'true');
 
@@ -608,36 +608,21 @@ export function form(id) {
 						}
 
 						if (result.type === 'result') {
-							array = /** @type {StandardSchemaV1.Issue[]} */ (
+							array = /** @type {InternalRemoteFormIssue[]} */ (
 								devalue.parse(result.result, app.decoders)
 							);
 						}
 					}
 
 					if (!includeUntouched && !submitted) {
-						array = array.filter((issue) => {
-							if (issue.path !== undefined) {
-								let path = '';
-
-								for (const segment of issue.path) {
-									const key = typeof segment === 'object' ? segment.key : segment;
-
-									if (typeof key === 'number') {
-										path += `[${key}]`;
-									} else if (typeof key === 'string') {
-										path += path === '' ? key : '.' + key;
-									}
-								}
-
-								return touched[path];
-							}
-						});
+						array = array.filter((issue) => touched[issue.name]);
 					}
 
-					const is_server_validation = !validated?.issues;
-					const new_issues = flatten_issues(array, is_server_validation);
+					const is_server_validation = !validated?.issues && !preflightOnly;
 
-					issues = is_server_validation ? new_issues : merge_with_server_issues(issues, new_issues);
+					raw_issues = is_server_validation
+						? array
+						: merge_with_server_issues(form_data, raw_issues, array);
 				}
 			},
 			enhance: {

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

@@ -116,39 +116,58 @@ export function deep_set(object, keys, value) {
 }
 
 /**
- * @param {readonly StandardSchemaV1.Issue[]} issues
- * @param {boolean} [server=false] - Whether these issues come from server validation
+ * @param {StandardSchemaV1.Issue} issue
+ * @param {boolean} server Whether this issue came from server validation
  */
-export function flatten_issues(issues, server = false) {
+export function normalize_issue(issue, server = false) {
+	/** @type {InternalRemoteFormIssue} */
+	const normalized = { name: '', path: [], message: issue.message, server };
+
+	if (issue.path !== undefined) {
+		let name = '';
+
+		for (const segment of issue.path) {
+			const key = /** @type {string | number} */ (
+				typeof segment === 'object' ? segment.key : segment
+			);
+
+			normalized.path.push(key);
+
+			if (typeof key === 'number') {
+				name += `[${key}]`;
+			} else if (typeof key === 'string') {
+				name += name === '' ? key : '.' + key;
+			}
+		}
+
+		normalized.name = name;
+	}
+
+	return normalized;
+}
+
+/**
+ * @param {InternalRemoteFormIssue[]} issues
+ */
+export function flatten_issues(issues) {
 	/** @type {Record<string, InternalRemoteFormIssue[]>} */
 	const result = {};
 
 	for (const issue of issues) {
-		/** @type {InternalRemoteFormIssue} */
-		const normalized = { name: '', path: [], message: issue.message, server };
-
-		(result.$ ??= []).push(normalized);
+		(result.$ ??= []).push(issue);
 
 		let name = '';
 
 		if (issue.path !== undefined) {
-			for (const segment of issue.path) {
-				const key = /** @type {string | number} */ (
-					typeof segment === 'object' ? segment.key : segment
-				);
-
-				normalized.path.push(key);
-
+			for (const key of issue.path) {
 				if (typeof key === 'number') {
 					name += `[${key}]`;
 				} else if (typeof key === 'string') {
 					name += name === '' ? key : '.' + key;
 				}
 
-				(result[name] ??= []).push(normalized);
+				(result[name] ??= []).push(issue);
 			}
-
-			normalized.name = name;
 		}
 	}
 

packages/kit/test/apps/basics/src/routes/remote/form/preflight-only/+page.svelte

@@ -0,0 +1,39 @@
+<script>
+	import { get, set } from './form.remote.js';
+	import * as v from 'valibot';
+
+	const data = get();
+
+	const schema = v.object({
+		a: v.pipe(v.string(), v.maxLength(7, 'a is too long')),
+		b: v.string(),
+		c: v.string()
+	});
+</script>
+
+<!-- TODO use await here once async lands -->
+{#await data then { a, b, c }}
+	<p>a: {a}</p>
+	<p>b: {b}</p>
+	<p>c: {c}</p>
+{/await}
+
+<hr />
+
+<form
+	{...set.preflight(schema)}
+	oninput={() => set.validate({ preflightOnly: true })}
+	onchange={() => set.validate()}
+>
+	<input {...set.fields.a.as('text')} />
+	<input {...set.fields.b.as('text')} />
+	<input {...set.fields.c.as('text')} />
+
+	<button>submit</button>
+</form>
+
+<div class="issues">
+	{#each set.fields.allIssues() as issue}
+		<p>{issue.message}</p>
+	{/each}
+</div>

packages/kit/test/apps/basics/src/routes/remote/form/preflight-only/form.remote.ts

@@ -0,0 +1,19 @@
+import { form, query } from '$app/server';
+import * as v from 'valibot';
+
+let data = { a: '', b: '', c: '' };
+
+export const get = query(() => {
+	return data;
+});
+
+export const set = form(
+	v.object({
+		a: v.pipe(v.string(), v.minLength(3, 'a is too short')),
+		b: v.pipe(v.string(), v.minLength(3, 'b is too short')),
+		c: v.pipe(v.string(), v.minLength(3, 'c is too short'))
+	}),
+	async (d) => {
+		data = d;
+	}
+);

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

@@ -1841,6 +1841,31 @@ test.describe('remote functions', () => {
 		}
 	});
 
+	test('form preflight-only validation works', async ({ page, javaScriptEnabled }) => {
+		if (!javaScriptEnabled) return;
+
+		await page.goto('/remote/form/preflight-only');
+
+		const a = page.locator('[name="a"]');
+		const button = page.locator('button');
+		const issues = page.locator('.issues');
+
+		await button.click();
+		await expect(issues).toContainText('a is too short');
+		await expect(issues).toContainText('b is too short');
+		await expect(issues).toContainText('c is too short');
+
+		await a.fill('aaaaaaaa');
+		await expect(issues).toContainText('a is too long');
+
+		// server issues should be preserved...
+		await expect(issues).toContainText('b is too short');
+		await expect(issues).toContainText('c is too short');
+
+		// ...unless overridden by client issues
+		await expect(issues).not.toContainText('a is too short');
+	});
+
 	test('form validate works', async ({ page, javaScriptEnabled }) => {
 		if (!javaScriptEnabled) return;