Skip to content
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

Fix subForm with no validators #22

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pedrodim
Copy link
Member

All the tests pass, it works now even if you validate only part of the subForm or none at all.

Having said that, I still not sure the fix is safe, the cast seems weird to me... or maybe is the return type of validateSubfield that's wrong 😄

@kanbanbot kanbanbot added the WIP label Dec 21, 2022
@@ -560,7 +560,7 @@ export function useFormo<
subfield: subfieldName,
errors: undefined,
});
return success(values[name]);
return success((values as unknown as V)[name][index][subfieldName] as Values[K]);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as Values[K] is needed because the function expects it to be that.

function validateSubfield<
    SK extends keyof SubFormValues<Values> & string,
    K extends SubFormKeys<Values>,
    V extends ArrayRecord<Values, SK>
  >(
    name: K,
    index: number,
    subfieldName: SK,
    values: Values
  ): Promise<Result<NonEmptyArray<FieldError>, Values[K]>>

The return type should be something like "Promise<Result<NonEmptyArray<FieldError>, ArrayItem<Values[K]>[SK]>>" as that's what we're actually returning.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah uhm, I see what you mean, Values[K] does not seem right (still it is being used and casted to in a lot of other places)

Copy link
Member

@giogonzo giogonzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @pedrodim!

The added tests look good to me, as well as the shady cast: there might be a better way to rewrite the function overall and remove the need for casting, but adding one more (same as others already used in the same function) seems ok for now

@kanbanbot kanbanbot added in review and removed WIP labels Dec 21, 2022
@giogonzo giogonzo self-requested a review December 21, 2022 19:04
@giogonzo
Copy link
Member

I'll take a better look tomorrow as I agree with you there is something weird going on with the subform validation typings 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants