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

Provide the resolver function with the name of the field that was changed #3110

Closed
vangelov opened this issue Oct 7, 2020 · 11 comments · Fixed by #3741
Closed

Provide the resolver function with the name of the field that was changed #3110

vangelov opened this issue Oct 7, 2020 · 11 comments · Fixed by #3741
Labels
feature request request a feature to be added

Comments

@vangelov
Copy link

vangelov commented Oct 7, 2020

Is your feature request related to a problem? Please describe.
I have a form containing a field array and each item in it is a form (let's call them subforms) containing around 30 fields. I'd like use onChange mode for validation as it provides immediate feedback but this expectedly causes performance issues when the subforms are more than 5.

The general Yup schema goes something like this:

const schema = object().shape({ subforms: array().of(getSubformSchema()) })

Describe the solution you'd like
The validation for each subform mentioned above is actually independent of the others. So when react-hook-form calls the validation resolver it can pass the name of the field that was changed (this will be undefined if the resolver is not called as a result of a field change). Using this name we can potentially optimise how the form is validated if the validation schema library supports it.

For example Yup provides the method validateAt which can validate a deeply nested schema. So when the resolver function is called with a field name we can use that name to find the parent subform of the field and only validate it, not the other subforms.

In general, I think having access to the name of the field that was changed and caused the resolver to be called, could be useful for other cases too.

Describe alternatives you've considered
Having multiple useForm calls, but this is hard as useForm is a hook and the count of the subforms is dynamic.

@bluebill1049 bluebill1049 added feature request request a feature to be added waiting-up-vote Waiting for votes from the community. labels Oct 7, 2020
@bluebill1049
Copy link
Member

good idea, do you have API proposal? probably need to aware other resolver's capability too, but this would be great for perf and i am putting this request on the board.

@bluebill1049 bluebill1049 added this to Pending Feature Requests in React Hook Form Oct 7, 2020
@vangelov
Copy link
Author

vangelov commented Oct 8, 2020

Could the name of the field be in the context object (the second parameter for the resolver function)? If not, the field name could be the second parameter (because context can be undefined) and context becomes the third one. Something like this: resolver: (values: any, fieldName: string?, context?: object) => Promise<ResolverResult> | ResolverResult

@bluebill1049
Copy link
Member

Could the name of the field be in the context object (the second parameter for the resolver function)? If not, the field name could be the second parameter (because context can be undefined) and context becomes the third one. Something like this: resolver: (values: any, fieldName: string?, context?: object) => Promise<ResolverResult> | ResolverResult

thanks, probably have to be the third argument, otherwise, I would be a breaking change. looks like most of the hard work will occur at the resolver level as well.

@vangelov
Copy link
Author

vangelov commented Oct 8, 2020

Ah, yes, the field name can actually be undefined, so it's fine to be after the context and not introduce any breaking changes.

i am putting this request on the board.

Does that mean it's going to get implemented or you need more people to vote for this?

@bluebill1049
Copy link
Member

Ah, yes, the field name can actually be undefined, so it's fine to be after the context and not introduce any breaking changes.

i am putting this request on the board.

Does that mean it's going to get implemented or you need more people to vote for this?

let's give it sometimes see if this feature gets some attraction first, 👍 I would probably follow a twitter poll as well.

@ntilwalli
Copy link

thanks, probably have to be the third argument, otherwise, I would be a breaking change. looks like most of the hard work will occur at the resolver level as well.

@bluebill1049 Wouldn't it be a fourth argument given that the current api already passes three? Or am I misreading this code?

const { errors } = await resolverRef.current!(
getValues(),
contextRef.current,
isValidateAllFieldCriteria,
);

@bluebill1049
Copy link
Member

oops, you are right @ntilwalli, I am thinking of introducing into the context, so context always has the field name.

@ntilwalli
Copy link

@bluebill1049 I get why you closed #3144 but just for clarity, this request is slightly different. I want the names that are passed as an argument to executeSchemaOrResolverValidation to be passed back to the resolver. Whether it's passed along with the field name in the context, or as a fourth argument doesn't really matter. I just want access to the names metadata so I can tailor the validation schema based on what validation is being requested.

@bluebill1049
Copy link
Member

@bluebill1049 I get why you closed #3144 but just for clarity, this request is slightly different. I want the names that are passed as an argument to executeSchemaOrResolverValidation to be passed back to the resolver. Whether it's passed along with the field name in the context, or as a fourth argument doesn't really matter. I just want access to the names metadata so I can tailor the validation schema based on what validation is being requested.

what do you mean names? you want the entire form input names?

@bluebill1049
Copy link
Member

The more user request attribute from resolver, the more I think those could probably be a part of the context object.

@ntilwalli
Copy link

ntilwalli commented Oct 11, 2020

I mean the names parameter in line 363 below. I'd like that value passed in somewhere in the function call on line 367, context or 4th argument. The names parameter is the set of fields that was passed into the call to trigger.

const executeSchemaOrResolverValidation = React.useCallback(
async (
names:
| InternalFieldName<TFieldValues>
| InternalFieldName<TFieldValues>[],
) => {
const { errors } = await resolverRef.current!(
getValues(),
contextRef.current,
isValidateAllFieldCriteria,
);

@bluebill1049 bluebill1049 moved this from Pending Feature Requests to API proposal / RFC in React Hook Form Dec 19, 2020
@bluebill1049 bluebill1049 moved this from API proposal / RFC to In progress in React Hook Form Jan 3, 2021
@bluebill1049 bluebill1049 mentioned this issue Jan 17, 2021
17 tasks
@bluebill1049 bluebill1049 moved this from In progress to Done in React Hook Form Mar 4, 2021
@bluebill1049 bluebill1049 removed the waiting-up-vote Waiting for votes from the community. label Mar 13, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature request request a feature to be added
Projects
Development

Successfully merging a pull request may close this issue.

3 participants