react-hook-form: Validation does not work correctly on Material-ui input with key prop

@bluebill1049 I have the same issue while using Material-UI with react-form-hook. After debugging, I found that the issue is a side effect from method findRemovedFieldAndRemoveListener().

For example, in sample codesandbox of @ASHFAQPATWARI, when switching language, Login component will be re-rendered and method registerIntoFieldRef will be triggered too.’

https://github.com/bluebill1049/react-hook-form/blob/88e29b696f265ffdafd329e038dc0842f75a9dd2/src/useForm.ts#L379

But at this time, fieldsRef already has value (referenced to old HTML Input element of email and password) therefore fieldsRef is not updated.

Right after the execution of registerIntoFieldRef(), JSS library (used by Material-UI) is executed to update style for input component but it also triggers callback on MutationObserver from method onDomRemove().

https://github.com/bluebill1049/react-hook-form/blob/681fd2f824f116dd749075ce42d01f04a7e009b5/src/utils/onDomRemove.ts#L3

Because the HTML Input element from parameter of onDomRemove() is the old element that does not exist in the current rendering page, method isDetached() returns true and onDetachCallback() will be triggered and this callback is findRemovedFieldAndRemoveListener().

https://github.com/bluebill1049/react-hook-form/blob/681fd2f824f116dd749075ce42d01f04a7e009b5/src/utils/onDomRemove.ts#L19

At this time, findRemovedFieldAndRemoveListener() will remove all values in fieldsRef.

https://github.com/bluebill1049/react-hook-form/blob/681fd2f824f116dd749075ce42d01f04a7e009b5/src/logic/findRemovedFieldAndRemoveListener.ts#L5

Now all values in fieldsRef has been removed, when clicking on submit button, method handleSubmit() always gets empty values from method getFieldsValues() and causes the wrong validation.

https://github.com/bluebill1049/react-hook-form/blob/681fd2f824f116dd749075ce42d01f04a7e009b5/src/useForm.ts#L537

In summary, the root cause is from findRemovedFieldAndRemoveListener() which is executed on old data by MutationObserver on document when style is changed.

A workaround is to put unSubscribe() inside Login component to clean up old data before re-rendering.

Updated working sample: https://codesandbox.io/s/react-hook-form-issue-on-first-click-ffhr0

@bluebill1049 Nice trick to update ref 👍 Actually, we can just use “lang” of Login component props as dependency for useEffect.

const Login = ({lang}) => {
    ...
    const [, forceUpdate] = React.useState({});

    React.useEffect(() => forceUpdate({}), [lang]);
    ...
};

Ok here is a solution for “hacking” key value with material-ui

Problem: (thanks @leapful based on your investigation)

• toggle key value on the same component
• confused React to remove the existing Element and then insert a new one (due to tree diffing [virtual DOM])
• confused React hook form because the name is the same however the actual dom is removed.
• when the element gets removed React hook form will remove all event listeners associated with the element to avoid memory leak
• re-render happened from context (provider), register gets invoked with old ref then React work out tree diff, remove the old node and insert the new one. However register is always called before that (when new DOM inserted) gets happened not after, hence registered ref lead to be the deleted one and empty.

Solution

• called hidden API unsubscribe on each render, which was suggested by @leapful it works, but terrible from the performance point of view, also his hidden API have been removed in the newest version.

• manually trigger a re-render after context update to make sure reigster get the correct ref after React reconciliation with the diffing, remove and insert. https://codesandbox.io/s/react-hook-form-issue-on-first-click-uxvhx

Conclusion

ideally, material-ui should fix this bug on the context API, and should stop users from toggling key on the same component to working around the re-render issue. This is definitely not standard to toggle key on the same component at least from my exp.

wow thanks, @leapful for the detailed investigation! I would love to get this working as well.

However, toggling individual element with the key is definitely not standard (edge case).

I will be looking into this issue see if I can work it out. Feel free to send a PR if you work out a solution too. much appreciated your investigation.

I think the problem is when key changed, React will detect a tree mismatch and removed the old element and inject with a new element. however, the input name still remains the same which lead register method believe it’s the same input, hence the issue.

(By the way, for the new version I have already removed unSubscribe from public api, so probably shouldn’t rely on this API)

really good investigation tho! 🙏 I didn’t even discover that.

(I may have a solution, going to try it out)

I don’t believe the key prop is the issue here. I’m able to reproduce it with the codesandbox provided. I’m going to see if I can figure out what is going on.

Debugging:

After changing language and the initial submit doesn’t trigger the getFieldValue but the second submit does. When the validateSchema function is called the first time, the data value is {}. The data value is empty due to the form not seeing any fields in the fieldsRef state. It looks like the refs are old. When inspecting the refs after language change, it doesn’t highlight the input. It happens after switching language, even switching from ar back to en and triggering submit. I think it may have to do with how material ui is handling the input ref. Something with how Material UI is handling their input ref is essentially causing us to have out of date refs. You can see the elementRef come into registerIntoFieldsRef correctly. However, It is doing a check if a ref with the same name already exists in it. So it is getting the previous ref.

@bluebill1049 I’m not sure if this is an issue with our lib, Material UI, or both? I suppose we could just overwrite the ref if it is different from the previous?

@bluebill1049 the key prop is used by material-ui to recalculate the inline styles as the label width change because the width of text is variable. There is nothing unusual with this as this is the recommended way by material-ui.

If I remove the key prop, then styling of my component will not work correctly.

ha, I find the issue. you didn’t give the button a type="button".

button without a type will automatically be assigned as submit 👍