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

useFetcher return value changes on every render #5113

Closed
dmarkow opened this issue Jan 16, 2023 · 4 comments · Fixed by #5118
Closed

useFetcher return value changes on every render #5113

dmarkow opened this issue Jan 16, 2023 · 4 comments · Fixed by #5118
Labels
bug Something isn't working

Comments

@dmarkow
Copy link
Contributor

dmarkow commented Jan 16, 2023

What version of Remix are you using?

1.10.1

Steps to Reproduce

https://github.com/remix-run/remix/blob/main/packages/remix-react/components.tsx#L1149

As of 1.10, Remix's useFetcher wrapper now returns a new object every time it's called. The means that using it in a hook's dependency array (like useEffect) causes the effect to re-run for every render, even when the fetcher's actual state hasn't changed.

Before 1.10, I could use it in useEffect and check for something like if (fetcher.type === "done") { ... } inside the effect. I was able to rely on the fetcher only causing the effect to re-run if something actually changed with the fetcher. But now, if the callback inside that useEffect does anything to cause a re-render, the useEffect falls into an infinite loop. Upgrading to 1.10 broke many pages on our site due to this. Unfortunately, because there are no success/error callbacks with useFetcher (yet?), I don't know any way other than useEffect to deal with the results of a fetcher.

For now, I can change all of my callbacks (unfortunately there are hundreds of them) to use fetcher.type in the dependency array instead of the whole fetcher object, but it would be nice if useFetcher wrapped its return object in a useMemo. I can send a PR for this if needed, but wanted to make sure there wasn't a specific reason this isn't being done already?

Some other functions (such as useTransition) do wrap their return values in useMemo, and before the upgrade to the new router, Remix returned a useMemo from useFetcher: https://github.com/remix-run/remix/blob/v1.6.4/packages/remix-react/components.tsx#L1389

Expected Behavior

useFetcher should not return a new object every render when nothing related to the fetcher has actually changed.

Actual Behavior

useFetcher returns a new object every render.

@brophdawg11
Copy link
Contributor

This should be available in 1.11.0-pre.1 shortly once CI finishes if you want to confirm it fixes your issue 👍

@brophdawg11
Copy link
Contributor

Released in 1.11.0

@brophdawg11 brophdawg11 removed the awaiting release This issue has been fixed and will be released soon label Jan 18, 2023
@brophdawg11 brophdawg11 removed their assignment Jan 18, 2023
@krall12
Copy link

krall12 commented Jan 19, 2023

After upgrading to 1.11.0 I believe this issue still persists. Something with the fetchers is not consistent between 1.9.0 and 1.11.0. I'll need to do more investigating.

@machour
Copy link
Collaborator

machour commented Jan 19, 2023

@krall12 please provide some reproduce case in a new issue 🙏🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants