Fix #58 issue - indexOf undefined

[Gabrielfcs]

PR Tests

I did some tests that involve this change, but I suggest a more in-depth test!

Related issue

This change solves the issue #58

[LauraBeatris]

We might verify the field name before doing any calculations with it.

Using an inline condition in one of the functions, do not cover all the cases that are using that value.

For instance, the error will be undefined if the fieldName is not provided:

  const error = useMemo(() => {
    return errors[fieldName];
  }, [errors, fieldName]);

What do you think?

Using an inline condition in one of the functions, do not cover all the cases that are using that value.

[LauraBeatris]

Also, I see that your commit message isn't following the contribution guidelines, so the build process is failing.

Please refer to the contribution guidelines in order see details about valid scopes and subjects. 😀

[Gabrielfcs]

@LauraBeatris This condition prevents the useField, created for all TextInput, from giving errors for not finding the object's property with this undefined fieldName. This is a problem of the dot-object dependency, which does not deal with this error when searching for object properties, if it does not have a defined property name.

Here is my solution in the dot-object dependency repository: dot-object #56

As this error happens only when the field does not have a name, it should only have problems in this place. I also performed exhaustive tests to make sure that the problem had been corrected.

As for the commit, it was missing to put a subject and therefore bot did not accept it as valid, but other than that I don't see any errors regarding the change

[LauraBeatris]

@Gabrielfcs I don't see any errors related to the changes proposed in issue either! Also, thanks for making an issue in the dot-object repository.

But that's something that we may discuss.

For instance, you pointed out that this change prevents errors when the fieldName is not being passed to the hook, and indeed, that's correct.

But let's take a look into the functions that are using the fieldName:

  const defaultValue = useMemo(() => {
	// Will not throw an expection and will skip to the calculation of the error value
    return fieldName && dot.pick(fieldName, initialData); 
  }, [fieldName, initialData]);

  const error = useMemo(() => {
    // With a undefined fieldName, it will return undefined
    return errors[fieldName];
  }, [errors, fieldName]);

  const clearError = useCallback(() => {
    // Passing a undefined value to another function 
    clearFieldError(fieldName);
  }, [clearFieldError, fieldName]);

What will happen if a component receives a defaultValue as undefined and try to access something inside it or pass it to another element/component?

The problem isn't related just with one calculation, it will also happen with the other functions mentioned above.

So we might add a conditional at the beginning of hook, refer to #249

[Gabrielfcs]

@LauraBeatris Okay, now I understand what you meant by other problems, but I don't think that using a throw at the beginning of everything will solve the problems, because as mentioned in this comment, there are cases in which we must let a field be used without having to go through the unform validations in a mandatory way. And firing an error goes against the freedom to use these components without using validations

[Gabrielfcs]

@LauraBeatris I don't know if this is the best way because I can't test it now, but maybe adding

if (!name) {
  return {registerField};
}

instead of

if (!name) {
  throw new Error('You need to provide the "name" prop.');
}

that was made in #249 fixes the problem.

[LauraBeatris]

@Gabrielfcs Good catch!

And firing an error goes against the freedom to use these components without using validations

But if the field it's not using validation and doesn't have a name in order to supply for form context to register a valid value and ref, there's no need for using the hook.

---

I don't know if this is the best way because I can't test it now, but maybe adding

How are you going to register a field in the form context without a valid name?

And also, it should change the return types definitions of the hook.

[LauraBeatris]

Also, there's another point that is worth mentioning.

The exception being thrown right now it's not descriptive and some developers which started using this lib may be confused.

[Gabrielfcs]

@LauraBeatris I'm not sure now, because I can't remember, but it seems to me that when I tested this earlier, the provider registers all the children in the Form.tsx file, and at some point it may be firing the hooks inside useField for storing it inside the unform or something. If you can test to make sure this is not happening it would be great

[diego3g]

@Gabrielfcs can you redo your commit following our contribution guidelines? https://github.com/Rocketseat/unform/blob/master/.github/CONTRIBUTING.md#commiting

[Gabrielfcs]

@diego3g Yes, I can commit again using the guidelines, but I believe this is not yet the root of the problem

[diego3g]

@Gabrielfcs I'll read all the discussion :+1:

[Laylsons3]

Estou com o mesmo problema. =/