-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix: improve fields type for generic components
#14974
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 0f984c1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
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>> |
fields type for generic components
|
@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 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. |
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
After the update to the
fieldstype in #14893, a type error occurs in generic<Form>components on theform.fieldsproxy. The.allIssues()method does not exist on the type and attempting to.set(data)yields a type error thatdatais not compatible despite being the same type as the form's input type.Current workaround is to assign
fieldsto a variable and assert it asRemoteFormFields<unknown>. But that reveals a separate issue, which I've proposed a fix for in #14973.Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits