kit icon indicating copy to clipboard operation
kit copied to clipboard

fix: improve `fields` type for generic components

Open sillvva opened this issue 4 months ago • 4 comments

Follow up to #14893

This PR reintroduces the original solution I had proposed in #14893. A suggestion was made in that PR to simplify the type, but that appears to have been a mistake as it caused issues with generic components.

Included in this PR is a test that would otherwise fail - a function that represents a generic form component

function f10<
	Schema extends StandardSchemaV1<RemoteFormInput, unknown>,
	Form extends RemoteForm<StandardSchemaV1.InferInput<Schema>, unknown>
>(data: StandardSchemaV1.InferInput<Schema>, form: Form) {
	form.fields.set(data); // data is incompatible
	form.fields.allIssues(); // allIssues() does not exist
}
void f10;

After the update to the fields type in #14893, a type error occurs in generic <Form> components on the form.fields proxy. The .allIssues() method does not exist on the type and attempting to .set(data) yields a type error that data is not compatible despite being the same type as the form's input type.

Current workaround is to assign fields to a variable and assert it as RemoteFormFields<unknown>. But that reveals a separate issue, which I've proposed a fix for in #14973.

const fields = form.fields as RemoteFormFields<unknown>;

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • [x] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • [x] This message body should clearly illustrate what problems it solves.
  • [x] Ideally, include a test that fails without this PR but passes with it.

Tests

  • [x] Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • [x] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Edits

  • [x] Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

sillvva avatar Nov 23 '25 22:11 sillvva

🦋 Changeset detected

Latest commit: 0f984c11a605c5be2859deab7ed23a485682e655

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sveltejs/kit Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar Nov 23 '25 22:11 changeset-bot[bot]

Preview: https://svelte-dev-git-preview-kit-14974-svelte.vercel.app/

svelte-docs-bot[bot] avatar Nov 23 '25 22:11 svelte-docs-bot[bot]

I also created a type for better intellisense. The question is whether to export it.

Intellisense now shows:

const fields: RemoteFormFieldsRoot<StandardSchemaV1.InferInput<Schema>>

instead of:

const fields: IsAny<StandardSchemaV1.InferInput<Schema>> extends true ? RecursiveFormFields : StandardSchemaV1.InferInput<Schema> extends void ? {
    issues(): RemoteFormIssue[] | undefined;
    allIssues(): RemoteFormIssue[] | undefined;
} : RemoteFormFields<StandardSchemaV1.InferInput<Schema>>

sillvva avatar Nov 24 '25 01:11 sillvva

@dummdidumm when you get a chance, can you take a look at this one? You had previously approved #14893 and this is a followup to that one to fix generic components/functions.

Here's an example of a function that needs as any to ignore the incorrect type errors.

export function initForm<Input extends RemoteFormInput>(form: RemoteForm<Input, unknown>, getter: () => Input) {
	let hydrated = false;
	// eslint-disable-next-line @typescript-eslint/no-explicit-any
	form.fields.set(getter() as any);
	$effect(() => {
		const values = getter();
		if (!hydrated) return void (hydrated = true);
		// eslint-disable-next-line @typescript-eslint/no-explicit-any
		form.fields.set(values as any);
	});
}

#14973 was also discovered when testing workarounds for this.

sillvva avatar Dec 08 '25 20:12 sillvva