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

useForm methods are not stable #1319

Closed
JamesHemery opened this issue Oct 16, 2022 · 3 comments
Closed

useForm methods are not stable #1319

JamesHemery opened this issue Oct 16, 2022 · 3 comments
Labels
react Related to the react adapter

Comments

@JamesHemery
Copy link

Versions:

  • @inertiajs/inertia version: 0.11.1
  • @inertiajs/inertia-react version: 0.8.1

Describe the problem:

The methods exposed by the return of the useForm hook are not stable. At each rendering, they are new functions and thus new javascript references.

This is a problem when we want to use one of these methods (reset, setData, ...) with a useEffect, useCallback, useMemo.

These hooks (useEffect, useCallback, useMemo) make superficial comparisons (based on references) to know if they should be executed.

Note that all variables created within a component and used in one of these hooks must be indicated in its dependency table.

Steps to reproduce:

Use this code with an input or anything that will trigger a re-render and the useEffect will be executed at each render. While however it should be executed only when reset method changes.

    const { data, setData, post, processing, errors, reset } = useForm({
        name: '',
    });

    useEffect(() => {
       console.log('This will be executed at each render');
    }, [reset]);

Short term fix

Remove the method from dependencies array but it's a bad thing.

    useEffect(() => {
       console.log('This will be executed at each render');
    }, []);

Long term fix

Memoize useForm returned methods with useCallback.

If you're ok (@reinink @claudiodekker) I can make a PR, but I have the impression that they are currently no reviewed?

@JamesHemery JamesHemery added the react Related to the react adapter label Oct 16, 2022
@NicolasKulka
Copy link

+1

Do you have a solution temporarly for this problem with useState or other ?

@JamesHemery
Copy link
Author

@NicolasKulka This is a patch (use patch-package) for Inertia React 0.8.1: https://gist.github.com/JamesHemery/9911ce1c9a8c39ea6bf383a8ac4aa8b9.

I need to take the time to suggest a PR.

@reinink
Copy link
Member

reinink commented Jul 28, 2023

Hey! Thanks so much for your interest in Inertia.js and for sharing this issue/suggestion.

In an attempt to get on top of the issues and pull requests on this project I am going through all the older issues and PRs and closing them, as there's a decent chance that they have since been resolved or are simply not relevant any longer. My hope is that with a "clean slate" me and the other project maintainers will be able to better keep on top of issues and PRs moving forward.

Of course there's a chance that this issue is still relevant, and if that's the case feel free to simply submit a new issue. The only thing I ask is that you please include a super minimal reproduction of the issue as a Git repo. This makes it much easier for us to reproduce things on our end and ultimately fix it.

Really not trying to be dismissive here, I just need to find a way to get this project back into a state that I am able to maintain it. Hope that makes sense! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
react Related to the react adapter
Projects
None yet
Development

No branches or pull requests

3 participants