react-final-form icon indicating copy to clipboard operation
react-final-form copied to clipboard

Register only once for useField hook

Open ryank109 opened this issue 5 years ago • 7 comments

Hi, I'm trying to address an issue describe here on issue #667 to improve performance on initial registration of the field.

Also there's a final-form PR that goes with this change: https://github.com/final-form/final-form/pull/319

Again, to describe the findings, with useField hook, it was using registerField to register for the first time to get the initial values synchronously, then unregister to throw away the initial subscription to get the initial state, then register again for subscribing to the subsequent updates to the form. And each time it registered, it fired off notify subscription on all previously registered fields and for the current subscription. So, if you are trying to render 100 fields on the screen, that's 2 registration per field and 10100 or n(n + 1) notifications.

So I wanted to separate the registration and subscription to make sure that registration and initial state is calculated during that registration, and subscribe in a separate call to the field states. With this change, I've reduced the number of notifications to 0, and only registering the field once. Another side effect of this change was the fix for this issue as well: https://github.com/final-form/react-final-form/issues/639. The change I made does not unregister the field when it's unmounted.

But..., since I wanted to make sure that it works with currently supported behaviors, I did introduced keepFieldStateOnUnmount property to useField/Field to make sure that the current logic of removing/unregistering the field on unmount still works.

ryank109 avatar Jan 31 '20 04:01 ryank109

Can you see if this is still a problem after v6.4.0?

erikras avatar Mar 31 '20 10:03 erikras

@erikras Yes, seems like it's still a problem. I've created a repo to test the performance of initial registering: https://github.com/ryank109/react-final-form-pref

[email protected] with 3000 fields ~7-8s with this fix, with 3000 fields ~1s

[Edit - 4/6/2020] To elaborate more, I think silent mode that you added on the first setState initialization did improve the performance a little from the previous version, because you narrow down the subscription call on the first register call. However, the useEffect call that comes right after the setState is registering and still calling to notify every subscription registered so far. Which is still a large operation if there's many, many fields registered on the screen.

Could you explain the reason behind why notifications on register matters? Maybe I'm not understanding the case that you were trying to cover. Still, I was make all the unit test pass without calling to notify subscriptions.

Thanks.

P.S. I'll update this PR after your feedback, because I'm not sure with my changes silent mode is necessary or not. Thanks!

ryank109 avatar Apr 04 '20 21:04 ryank109

@erikras Any recommendations?

ryank109 avatar Apr 19 '20 01:04 ryank109

@erikras Do you have any objections against this functionality? I'm asking since i want to finish work with this PR and if possible merge it.

ra2dev avatar Apr 30 '22 21:04 ra2dev

@ra2dev What is the status of this PR. It will be really useful to me, because I have a form, separated in different tabs? My current workaround if anyone curious, is to keep registered all Form fields, all the time rendering null.

@erikras Since FF is not actively maintained, can to transfer rights to anyone who wants.

Dragomir-Ivanov avatar Sep 13 '22 13:09 Dragomir-Ivanov

@Dragomir-Ivanov Was not able to dedicate time on this PR, resolved issue on my side by creating separate context to track registered fields that are located on "hidden" tabs (temp solution). Next month planning to work on blog post related to performance of form libraries, hopefully will be able to play with it and mb provide solution

ra2dev avatar Sep 15 '22 21:09 ra2dev

Thank you @ra2dev , and sorry for the late response. The issue with "hidden" fields, is that being hidden they don't loose their value, but loose their state, i.e.: dirty, touched, modified, etc.. This makes working with FF hacky. It seems that one can't just hold a reference to a FormApi, without any form rendering, and persist, form state. I am already thinking in favor of React Form Hook, and will try to swap on first occasion. Please include RFH in your blog as well.

Dragomir-Ivanov avatar Sep 21 '22 13:09 Dragomir-Ivanov