conform icon indicating copy to clipboard operation
conform copied to clipboard

Support validate intent to validate a full fieldset (all fields with a given prefix)

Open oxc opened this issue 1 year ago • 8 comments

Proposed fix for #828

Here is a sandbox with the same example from above Issue, where this patch is applied, and a second button is added to the task fieldset that demonstrates the behaviour: https://codesandbox.io/p/devbox/sad-tree-fdj7hj?workspaceId=ws_3qoruoNJ3FvmBgCfSAACYX

oxc avatar Dec 07 '24 13:12 oxc

Open in Stackblitz

More templates

@conform-to/dom

npm i https://pkg.pr.new/@conform-to/dom@830
@conform-to/react

npm i https://pkg.pr.new/@conform-to/react@830
@conform-to/validitystate

npm i https://pkg.pr.new/@conform-to/validitystate@830
@conform-to/yup

npm i https://pkg.pr.new/@conform-to/yup@830
@conform-to/zod

npm i https://pkg.pr.new/@conform-to/zod@830

commit: 26a4346

pkg-pr-new[bot] avatar Dec 07 '24 13:12 pkg-pr-new[bot]

Hi @oxc, thanks for the PR! I can see this solution works for many cases. But the errors object might not have every fields and so there will be some minor inconsistency depending on the validation logic. Ideally, I think we need a different data structure so we can express is as every fields under x is validated, except y, z etc 🤔

But as a short term solution, how about making the validate intent accept an array of string? We can combine it with a simple utility to look for all the fields under it to achieve the same functionality, like:

form.validate({
  // This can be a string or an array of string
  name: ['address', 'address.city', ...],
});

form.validate({
  // Use a helper to look up fields from the DOM
  name: getFields(formElement, name),
});

// Example `getFields()` implementation
function getFields(formElement: HTMLFormElement, name: string) {
  const fields: string[] = [];

  for (const element of formElement.elements) {
    if (isInputElement(element) && element.name.startsWith(name)) {
       fields.push(element.name);
    }
  }

  return fields;
}

edmundhung avatar Feb 22 '25 16:02 edmundhung

Thanks, I will look into this an prepare a new PR.

oxc avatar Mar 10 '25 10:03 oxc

Hi @edmundhung, I looked into your suggestion. It will probably work for all my own use cases, but not all conceivable ones.

More specifically, it will not return all errors that will be returned for this fieldset when validate is called without name.

Especially, it will not report errors on missing values for which there was no field in the DOM, for example. Consider a nested array with its own insert intent button.

Nevertheless, I think it's useful to have this feature to validate multiple specific fields at the same time, so I will prepare a PR for it. EDIT: Created #878

oxc avatar Mar 12 '25 03:03 oxc

Would you think different about this PR if I renamed the new property from recursive to allErrors? That would make it more in line with how it will get used. If you pass allErrors: true, it will include all errors in the output that would be returned for this field that would be returned if you passed no field name. In fact, I still think this should perhaps be the default behaviour.

EDIT: Created #879

oxc avatar Mar 12 '25 03:03 oxc

More specifically, it will not return all errors that will be returned for this fieldset when validate is called without name.

I am not sure if I follow. I think that's what the array of names solves as I expect people to provide all the field names instead. Do you mean the expectation of validating without a name specified doesn't match?

Especially, it will not report errors on missing values for which there was no field in the DOM, for example. Consider a nested array with its own insert intent button.

This is interesting as it is intentional not to have the newly insert item being validated. Won't it be awkward if you insert a new field and we show the error of that right away? I feel like you have a specific use cases in mind which I might have missed. Can you elaborate more?

edmundhung avatar Mar 19 '25 04:03 edmundhung

More specifically, it will not return all errors that will be returned for this fieldset when validate is called without name.

I am not sure if I follow. I think that's what the array of names solves as I expect people to provide all the field names instead. Do you mean the expectation of validating without a name specified doesn't match?

More specifically, it will not return all errors that will be returned for this fieldset when validate is called without name.

I am not sure if I follow. I think that's what the array of names solves as I expect people to provide all the field names instead. Do you mean the expectation of validating without a name specified doesn't match?

Yes, exactly. When I validate a field set (like the Todo in the example), I expect it to show exactly the same errors it will show when I submit the full form. That's currently not possible.

Especially, it will not report errors on missing values for which there was no field in the DOM, for example. Consider a nested array with its own insert intent button.

This is interesting as it is intentional not to have the newly insert item being validated. Won't it be awkward if you insert a new field and we show the error of that right away? I feel like you have a specific use cases in mind which I might have missed. Can you elaborate more?

I might have described that badly. What I meant is: if you have a nested array with insert button, you only have field elements for the single entries, not for the array field itself.

Consider having such an array inside a field set, e.g. a hypothetical "labels" array for each task in the Todo list example.

If you have constraints on the array field ("Add at least one tag"), they won't be shown if you validate the surrounding field set.

They will, however, show once the full form is submitted. This is the discrepancy I would like to solve.

oxc avatar Mar 19 '25 06:03 oxc

I have create a StackBlitz based on the multi-validate PR (#878) that demonstrates the problem, based on your suggestion to collect the field names from the Form element:

https://stackblitz.com/edit/uzog2zz5?file=src%2Ftodos.tsx

  1. Open the Todos List example
  2. Add a Task
  3. Click the Clear button to enable the Validate button[^1]
  4. Click Validate button
  5. Note that only the Content field error shows up in the Task's error list
  6. Click the Save button at the bottom of the Form
  7. Note that now also the Error for the Tags array shows up in the Task's error list

[^1]: perhaps I did something wrong there, but I guess at the time of first rendering, there are no fields in the Form with the tasks' name prefix, which would be another drawback of your suggestion.

oxc avatar Mar 21 '25 15:03 oxc

As commented in https://github.com/edmundhung/conform/pull/878#issuecomment-3166049037, you will be able to validate nested fields in v2.

Let me know if you have any questions 🙌🏼

edmundhung avatar Aug 07 '25 22:08 edmundhung