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

unstable_Blocker is making update depth exceeded. #6704

Closed
1 task done
fernandojbf opened this issue Jun 27, 2023 · 13 comments · Fixed by #6737
Closed
1 task done

unstable_Blocker is making update depth exceeded. #6704

fernandojbf opened this issue Jun 27, 2023 · 13 comments · Fixed by #6737

Comments

@fernandojbf
Copy link
Contributor

What version of Remix are you using?

1.18.0

Are all your remix dependencies & dev-dependencies using the same version?

  • Yes

Steps to Reproduce

  1. create a new remix project (eg: npx create-remix@latest --template remix-run/indie-stack blog-tutorial)
  2. import unstable_useBlocker from @remix-run/react
  3. use it in a page unstable_useBlocker(() => false);

Expected Behavior

The console should be clean without any Warning: Maximum update depth exceeded.

Actual Behavior

Maximum update depth exceeded.

image

@fernandojbf
Copy link
Contributor Author

fernandojbf commented Jun 27, 2023

I believe the root cause of it is a using a key and setting a key in the useEffect -> https://github.com/remix-run/react-router/blob/843fe0e989b17c484d3cf6b54daf342f7dc65394/packages/react-router/lib/hooks.tsx#L980

@fernandojbf
Copy link
Contributor Author

Introduced by this remix-run/react-router#10573

@brophdawg11
Copy link
Contributor

The generation of a net-new function identity is the underlying cause here, since it's triggering a new blocker key generation. I'll look into a fix for this but the workaround for now is to wrap your blocker function in useCallback.

unstable_useBlocker(useCallback(() => false, []));

@brophdawg11
Copy link
Contributor

brophdawg11 commented Jun 29, 2023

This should be resolved in remix@1.18.1-pre.1 if you'd like to test it out!

@fernandojbf
Copy link
Contributor Author

thanks @brophdawg11

The generation of a net-new function identity is the underlying cause here, since it's triggering a new blocker key generation. I'll look into a fix for this but the workaround for now is to wrap your blocker function in useCallback.

Yap, I've missed read the blockerKey vs blockerId 🤦 .

Sorry if this does not make sense but:

Why do we have a state for the blocker?
Since initial blocker is always idle and we want to use the DataRouterStateHook more than internal state, i believe we do not need the internal state.

  React.useEffect(() => {
    if (blockerKey !== "") {
    // no need to setState since state DataRouterContext will give us any update (if no update IDLE_BLOCKER)
      router.getBlocker(blockerKey, blockerFunction);
    }
  }, [router, blockerFunction]);

  // Prefer the blocker from state since DataRouterContext is memoized so this 
  // ensures we update on blocker state updates
  return blockerKey && state.blockers.has(blockerKey) ? state.blockers.get(blockerKey) : IDLE_BLOCKER;

Other thing, and to be honest i don't know if would be a good solution:
Since the key is not needed for the initial render but only when DataRouterContext changes, we could use a useRef to store it instead of a state. since the useEffects run in order this would make everything done in one lifecycle.

@brophdawg11
Copy link
Contributor

Yeah that would probably be fine to remove the blocker/setBlocker - we were using local state just as a way to be sure we were following the rules of concurrent mode to get the key tracking right. I'll see about working that into a PR I'm working on now.

The ref I'm less sure about due to the nuances of Strict Mode double rendering. I'd be happy to look over a PR if you wanted to play around with it though!

@fernandojbf
Copy link
Contributor Author

Sure. I will open a PR and ping you for you to have a look and see if the problems that you where facing continue. I will ping you in a bit :)

@fernandojbf
Copy link
Contributor Author

I endup not making the ref solution because was more an hack than a solution. I believe the one i've create remix-run/react-router#10657 is more interesting

Cheers

@brophdawg11 brophdawg11 linked a pull request Jun 30, 2023 that will close this issue
@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 1.18.1-pre.2 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 1.18.1 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@devel0
Copy link

devel0 commented Jan 22, 2024

using react-router-dom 6.21.3 I hit the issue within useBlocker,
actually downgrading to 6.12.1 and using unstable_useBlocker the issue disappeared.

The behavior is that the first time I go back using browser back button the blocker works as expected even the log a router only support one blocker at a time then editing again in the form and hitting back button with proceed the browser starts to forward and backward indefinitely.

@brophdawg11
Copy link
Contributor

@devel0 Could you please open a new issue with a reproduction?

@devel0
Copy link

devel0 commented Jan 27, 2024

@brophdawg11 did it see mentioned issue above for the repro

simplescreenrecorder-2024-01-27_16.49.54.webm

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

Successfully merging a pull request may close this issue.

3 participants