issue: v7.22.5 brakes the way undefined form values are handled

[codeaid]

Version Number

=7.22.5

Codesandbox/Expo snack

https://stackblitz.com/edit/react-hook-form-bug?file=Form.tsx

Steps to reproduce

I have created a component in the sandbox that replicates behaviour of the Select (dropdown) component that I have in my project. The idea is nothing new - you have a list of options in a dropdown, each of them has a value and you just pick one of them. Additionally, you can have an option at the top which indicates no selection (e.g. "All" or "None" depending on the context), whose value is undefined instead of it being a string.

As an example - it could be a list of colors to filter something by, e.g. "Red", "Blue", "Green" and one option above "All", which would indicate that no specific color should be used. That "All" option's value is undefined.

To demonstrate the issue I'm having:

1. Run the sandbox and ensure that the react-hook-form library version is set to 7.22.4 in the dependency panel on the left (important!). If it's not, paste [email protected] into the package name field and press Enter.
2. Wait for the page to update and note that the Form component is being rendered with the following defaults in the index.tsx file (which are also reflected by the blue highlighted buttons in the panel on the right):

<Form defaultValue1="one" defaultValue2={undefined} />

3. Next, observe how you can toggle both of the options between "one", "two" and "undefined" back and forth without any issues (you can also open the console to see some additional debug output).
4. Now, change the version of the library to 7.22.5 by pasting [email protected] into the package name field and pressing Enter.
5. Wait for the page to reload and try repeating the same option changes as before.
6. Notice how the top option that defaulted to one cannot be toggled to undefined anymore yet the second option, whose default value was undefined can.

I don't know what changed exactly between these two minor versions but this has broken all of my dropdown components as there is now no way of selecting the "All" options if the initial value of the form field was anything else but undefined.

I tried a few versions post 7.22.5 and they all exhibit the same issue.

Expected behaviour

Form fields whose values can be 'undefined' should work the same way as they did prior to v7.22.5.

What browsers are you seeing the problem on?

Firefox, Chrome, Safari, Edge

Relevant log output

No response

Code of Conduct

• I agree to follow this project's Code of Conduct

[Moshyfawn]

Just to add some additional info, notice how with the "new" 7.22.5 behavior, the value1 Controlled component doesn't rerender, which leads to it's value not being set to undefined. As seen in the Values object log, the value1 form field itself does change to undefined.

undefined-across-versions.mp4

I'd suggest to use null value as the default "none" / "all" option instead. It consistently works across all RHF versions and goes along with JS definitions of values

[codeaid]

@Moshyfawn Using null would've been my next attempt but that means refactoring quite a bit of logic in my code, including changing various types from prop?: string to prop: string | null, which I really don't want to do.

I was hoping this undefined behaviour is actually a bug so curious to see what the author says before I start refactoring it all.

[Moshyfawn]

Basically, this check is what's breaking your use-case.

updateValue(
  isUndefined(_name.current) ||
    (isObject(fieldValues) && !objectHasFunction(fieldValues))
    ? { ...fieldValues }
    : Array.isArray(fieldValues)
    ? [...fieldValues]
>   : isUndefined(fieldValues)
>   ? defaultValue
>   : fieldValues,
);

If the new value is undefined, the field's defaultValue is used. You can see it fully if you select the option that isn't the default one (option "two" in your case) and then try to select the "undefined" one. The value is set to "one" instead.
Now, the question is: @bluebill1049 is it an edge-case or should the OP use null instead?

[bluebill1049]

undefined need to be avoided, it is used to fall back to defaultValue with defautlValues since hook form does allow the user to provide inline defaultValue/defaultChecked. The original fix was introduced to fix bug-related. See this PR for more detail: #7360 thank you @Moshyfawn for looking into this.

[zo-ly]

I have the same problem CodeSandBox

In MyInput, I can solve it through onChange(null), but I cannot clear value in MySelect

[bluebill1049]

I have the same problem CodeSandBox

In MyInput, I can solve it through onChange(null), but I cannot clear value in MySelect

It's a text input, right? why not an empty string?

[zo-ly]

I have the same problem CodeSandBox
In MyInput, I can solve it through onChange(null), but I cannot clear value in MySelect

It's a text input, right? why not an empty string?

yes, I can set an empty string or null value to clear the text input, but I cannot clear the select

record.mov

[Moshyfawn]

yes, I can set an empty string or null value to clear the text input, but I cannot clear the select
record.mov

That's exactly what you're doing: clearing the field to it's default value which is Role: 1. The Antd Select component passes undefined to onChange callback when the clear event happens

You might wanna handle the undefined onChange like

onChange={(value) =>
  if (!value) {
    field.onChange(null)
  }
}

Or something along these lines

[zo-ly]

yes, I can set an empty string or null value to clear the text input, but I cannot clear the select
record.mov

That's exactly what you're doing: clearing the field to it's default value which is Role: 1. The Antd Select component passes undefined to onChange callback when the clear event happens

You might wanna handle the undefined onChange like

onChange={(value) =>
  if (!value) {
    field.onChange(null)
  }
}

Or something along these lines

In my scenario, null is an alternative value. Is there any other solution apart from changing null to 0, converting it to null and submiting it to the backend? I don't want null to be this complicated.

[Moshyfawn]

You don't need to convert null to anything. But you gonna need to check for the undefined value that Antd Select (the component lib that you're using) returns onChange every time you clear the input. RHF checks for undefined and sets the value to it's default value in case of undefined.

[zo-ly]

I know Antd Select returns undefined when I clear the input, it's just what I need.

RHF checks for undefined and sets the value to it's default value in case of undefined

That's what I'm trying to do: RHF do not sets the value to it's default value in case of undefined

[Moshyfawn]

That's what I'm trying to do: RHF do not sets the value to it's default value in case of undefined

That's the intended set behavior of RHF since #7360. Atm, there're no plans to change it, from what we can tell.
See #7341 for the discussion about the unwanted behavior you're experiencing.
There're ppl claiming that their use-case is the most common behavior. And there's a s*it load of different UI libraries doing their own thing like not exposing the input's ref which you need to set the focus on the input, or return undefined in your case, which might be an anti-pattern for other UI lib, so stuff breaks. We can't expect every lib to continue invent boolean values for each and every use-case there's.
At the end of the day, there're patterns and rules the lib is following that the maintainer has decided on. Discussions are welcome, of course. But it was clearly stated that this is the default behavior of the lib atm.

[MudulOzan]

this discussion saved my time, appreciate it guys, love ya <3

[bluebill1049]

Summary

• avoid undefined i believe it's troublesome even for controlled component
• undefined is what hook form try to check fallback value for inline vs useForm level defaultValues
• This behavior is by design

[codeaid]

@bluebill1049 My issue with this change in the PR you linked is that it's a breaking change in a patch (0.0.1, not even 0.1.0).

Going from undefined to null is a shift in paradigm and is most of the time not an easy fix because it's not even the same concept - one is a type representing emptyness and the other denotes a property that might not be present.

If there are components like mine (of which I'm sure there are many) that support a list of values or undefined then there obviously are even more consumers relying on the component emitting undefined and just flipping it to null causes a lot of work.

In my case I could probably refactor a large part of my codebase to use null instead of undefined and then convert all the nulls back to undefined again (because I have other code that expects prop?: string and not prop: string | null) but it's a big effort, plus it's in fact multiple applications that share the same components, not just one.

[bluebill1049]

I understand this could be frustrated to your application, but let me redeclare what I just stated above. This is by design and I have clearly documented this feature in the documentation.

https://react-hook-form.com/api/useform

It is encouraged that you set defaultValues for all inputs to non-undefined values such as the empty string or null.

I didn't make a breaking change, this is just a bug fix as you can trace it down to the original fix.

[codeaid]

This is by design and I have clearly documented this feature in the documentation.

It worked differently for seven and a bit major versions and the behaviour was changed in a patch, not even a minor version. People have given examples of popular component libraries taking the same approach yet the "solution" seems to be to rewrite large parts of codebases (or just lock the library to v7.22.4.

I didn't make a breaking change, this is just a bug fix as you can trace it down to the original fix.

Even a bugfix can be breaking existing functionality. How you call it doesn't matter.

P.S.
I would appreciate if you would please stop marking your reply as an answer to my question because it doesn't answer it.

[Moshyfawn]

What's the solution you're looking for, @codeaid?
From what I can tell, the way to solve it, so your app doesn't break, is to lock your RHF version to 7.22.4 for now.
As stated above, the unwanted behavior you're experiencing is the result of #7341 and is the intended behavior of RHF at least atm. You're welcome to provide your use-case and opinion on the matter, the have a discussion, ofc
Also, this

[codeaid]

@Moshyfawn The solution I was hoping to achieve when I initially posted the issue was to get the library to work the way it had worked for the last few years even if I upgraded it past 7.22.4. In my view features like this shouldn't have been changed in a patch (we have semantic versioning for a reason), it should've been a minor version change at the absolute minimum(!) because up until then undefined was a perfectly valid value that RHF supported. Bug or not it was how it worked since probably v1.0.

As it stands, it turns out RHF 7.22.5+ no longer supports elements or custom components that can have undefined as their value and users of this library have to implement workarounds of converting internal undefined to RHF's null and then back if they want to still use this library without having to tailor their custom components to support nulls. It's a lot of additional work, especially in large code bases so, as you suggested, all I can do at the moment is lock the version to 7.22.4 (not even ^7.22.4).

I'm sure you can understand how impactful this change is, not just in my library but in the whole approach that RHF takes and I think library's inability to natively handle undefined values should be documented very clearly going forward.

On a side note - I apologise if I sounded a bit impolite in some of my comments but I felt that the way this issue was handled was a bit rude (not you specifically) - instantly got converted to a discussion (which means that people browsing issues would not get to see it unless they unhide closed ones) and a comment from the maintainer essentially saying "That's how we implemented it, deal with it". I understand open source is free and I have no rights to demand anything but equally I also expect to at least have a dialog and not be shut down as soon as the owner logs in. Peace.

[codeaid]

Just to comment on a couple things:

lock your RHF version to 7.22.4 for now

There is no "for now", it's either lock it forever or refactor existing components to support v7.22.5+ going forward.

You're welcome to provide your use-case and opinion on the matter, the have a discussion, ofc

Which I did in my original issue description and a few other people did in their comments. I don't really know what else I am expected to supply.