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

trigger() behaves differently for normal fields and field arrays #2379

Closed
benswinburne opened this issue Jul 27, 2020 · 14 comments
Closed

trigger() behaves differently for normal fields and field arrays #2379

benswinburne opened this issue Jul 27, 2020 · 14 comments
Labels
question Further information is requested

Comments

@benswinburne
Copy link

Describe the bug
Calling trigger when no schema is in place passes normal fields (as expected) but fails array fields.

To Reproduce
Steps to reproduce the behavior:

  1. Go to the sandbox link
  2. Click on trigger with schema undefined. normal field passes, array field fails.
  3. Click on trigger with schema as schema. normal field passes, array field passes when criteria met.
  4. Click on trigger with schema as emptySchema. normal field passes, array field passes.

Codesandbox link (Required)
Include a codesandox will help us to investigate the issue quicker.

https://codesandbox.io/s/react-hook-form-usefieldarray-template-4ruie?file=/src/index.js

Expected behavior
I would expect the behaviour of trigger to be consistent for normal fields and array fields in the absence of a provided schema.

@bluebill1049
Copy link
Member

bluebill1049 commented Jul 27, 2020

That's expected behavior, when using scheme you are validating against the inputs with your schema, however for build-in validator and it's field level, you will have to trigger setError manually for minLengh array field length.

@benswinburne
Copy link
Author

In the absence of a schema they behave differently though. I know if i use a schema it works and how to validate it, but if one type of field passes when no schema and the other one fails then that's unexpected. I'd posit that they should both do the same, i.e. if no validation schema exists then all fields should pass validation.

trigger() on an array field when no validation is required should return true, not false.

When describing it logically it seems to make little sense.

is validation required on this field? no because there is no schema. does it validate? no.

If the argument an argument can be made for the above, then surely the same is true for normal non array field types.

currently a normal field follows the opposite logic.

is validation required on this field? no because there is no schema. does it validate? yes.

@bluebill1049 bluebill1049 reopened this Jul 27, 2020
@bluebill1049
Copy link
Member

Screen Shot 2020-07-27 at 7 46 34 pm

Yes, I understand your concern above. As today, we are not supporting key name trigger with trigger, rather supply an individual key name or array of the name`, it's working for schema because we are valid against the schema. So the solution, for now, you can use schema validation for validate against FieldArray as a whole, for build-in validate you will have to specify the key name or array of key names.

@bluebill1049 bluebill1049 added the question Further information is requested label Jul 27, 2020
@bluebill1049
Copy link
Member

bluebill1049 commented Jul 27, 2020

in the meantime, you can send a feature request for trigger(keyName) to support key names for build-in validate.

@benswinburne
Copy link
Author

benswinburne commented Jul 27, 2020

That's what I'm doing, I'm explicitly calling in trigger('test') and I expect it to pass if there is no schema, which it doesn't. I have to create logic to attempt to determine which kind of field/if there's schema on the form.

I am looking to pass validation on the name of a field when that field is an array if there is no validation set.

I'm not looking for trigger(keyname), that's a separate issue but makes sense to be able to do. I'm entirely looking for consistency in the output of trigger-

i.e. if there is no schema provided

field = array type
field2 = normal type

trigger() // true (currently true which is different to ['field', 'field2] which also validates all)
trigger('field') // true (currently false for some reason)
trigger('field2') // true (currently true as there's nothing to validate against))
trigger(['field', 'field2]); // true (currently returns false because field2 passes, field fails)

https://codesandbox.io/s/react-hook-form-usefieldarray-template-f3iv0?file=/src/index.js

In the above CSB you can see examples of calling trigger from your screenshot above.

A key point i suppose there is

trigger(['field', 'field2]) and trigger() both run against all fields in the form and return different results.

@bluebill1049
Copy link
Member

That's what I'm doing, I'm explicitly calling in trigger('test') and I expect it to pass if there is no schema, which it doesn't.

That test is a key not a field name.

@benswinburne
Copy link
Author

image

It is this field

@bluebill1049
Copy link
Member

image

It is this field

yes, 'test' is the key for the fieldArray, not the fieldName, fieldName is `test[0].xxx'.

@bluebill1049
Copy link
Member

bluebill1049 commented Jul 27, 2020

{test: [ { data: ''}, {data1: ''} ]}

  • test is the key name
  • test[0].data is the field name

right now trigger only support fieldName.

Screen Shot 2020-07-27 at 8 12 23 pm

@benswinburne
Copy link
Author

Right that makes a bit more sense! So it's the fact that RHF sees that field as absent.

https://codesandbox.io/s/react-hook-form-usefieldarray-template-cy2op?file=/src/index.js

Demo of an absent field fails too.

Thanks very much for your help and feedback there, that's cleared things up for me. Really appreciate how involved in the GH issues you are, it's really refreshing and no doubt a huge help to many.

Thanks again.

@bluebill1049
Copy link
Member

no worries mate, sorry I wasn't clear enough in the first reply, but please send us a feature request, I will log this in our project. Just be mindful, we are pretty cautious about the feature request because as you know each feature increases the package size. today we are still one of the small form libs in the industry. so be patient with us as we move/hold feature into progress, we tend to wait for community vote first as well.

@benswinburne
Copy link
Author

Of course, that makes sense, understandable. Where do people vote on features?

@bluebill1049
Copy link
Member

Screen Shot 2020-07-27 at 8 30 50 pm

#1981

@ewengillies
Copy link

I wrote a quick helper function for someone who wants to recursively trigger all objects in an array field. Its pretty simple:

  • It looks thru all the values in the getValues call.
  • It unpacks each object or array into in the return of getValues, recursively, to produce all the fieldName that exist in the same useForm data.
  • It filters the return of the recursion to fieldNames that start with the desired key.
  • this also works for nested fields/nested arrays.

The main downside is that it calls getValues each time, when all I really need is the "tree of field names" structure. Its not my best recursive function, but it works well enough for now:

const getAllNamesByKey = (values, key) => {
  // Choose which parser we are using, as arrays use [ ] notation in useForm
  const parseArray = (stack, property) => `${stack}[${property}]`;
  const parseObj = (stack, property) => `${stack}.${property}`;
  // Recursively unpack the keys
  const unpackKeys = (obj, stack, toReturn) => {
    // Check which parser we will use
    const parser = Array.isArray(obj) ? parseArray: parseObj;
    for (const property in obj) {
      if (obj.hasOwnProperty(property)) {
        const returnString = parser(stack, property);
        // If its an object, recurse
        if (typeof obj[property] == 'object') {
          unpackKeys(obj[property], returnString, toReturn);
        } else {
          // This has a leading "." in it due to the start step in the
          // recursion.  Remove it using slice.
          toReturn.push(returnString.slice(1));
        }
      }
    }
  };
  const toReturn = [];
  unpackKeys(values, '', toReturn);
  return toReturn.filter((fullKey) => fullKey.startsWith(key));
};

const triggerByKeyGenerate = (getValues, trigger) => (key) => {
  const values = getValues();
  const namesToTrigger = getAllNamesByKey(values, key);
  namesToTrigger.map((name) => trigger(name));
};

The structure above lets you build triggerByKey right where you instantiate {trigger, getValues, ...other} = useForm(). For example

const {handleSubmit, setValue, getValues, errors, control, trigger} = useForm();
const triggerByKey = triggerByKeyGenerate(getValues, trigger);
..... 

triggerByKey("highlevel[0]");  // trigger every field in the first element in the array
triggerByKey("highlevel");  // trigger every field in every element of the array, recursively

Hopefully this will save others some time if they want the same functionality. Thanks for the great library btw, love it!

@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
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants