reduce internal states in useBlocker

[fernandojbf]

This PR reduces the number of internal states used in useBlocker

I believe this solution works and brings less useEffects interactions with internal state.

With StrictMode we will continue to have 2 ids. But we will never have an orphan blocker. The first ID will be added and removed making the code more deterministic.

For the normal mode (no strict mode) the id generation only works once and the effect makes always the same action (adding and removing on unmount).

I believe this simplifies the solution.

[changeset-bot]

🦋 Changeset detected

Latest commit: c2e2ac1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
react-router	Patch
react-router-dom	Patch
react-router-dom-v5-compat	Patch
react-router-native	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[fernandojbf]

@brophdawg11 i end up not adding the ref solution (mention remix-run/remix#6704) because, like was mentioning before, I was not sure that I liked the ref solution.

I believe the useMemo in conjunction with the useEffect is more deterministic, not mattering the mode running.

Please can you check if the problems that made you open the first change continue to happen

Cheers

[remix-cla-bot]

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

[fernandojbf]

@brophdawg11 did you got time to test if this is working?

Cheers

[brophdawg11]

Thank you for the PR but I think we're going to remain with the current solution for now. The useMemo trick doesn't appear to break anything from a quick test but it also feels like a violation of https://react.dev/learn/keeping-components-pure since the act of rendering now has a side effect of incrementing the key.

This blocker stuff has been a bit fragile (as expected and why it's unstable_) so we don't want to introduce any unneccesary changes that may break things in unforeseen ways.

[fernandojbf]

Thanks for the reply.

Yap make sense. I thought could be a something possible since isHydrated seems to be used in a non pure way in the components file.

Without the useId, the useEffect would be the only way to do it maintaining the purity.

At the same time the idea of this component is to be impure (due to unique keys). We are just moving the impurity to the effect (like the documentation mention). For sure the render is not pure, but is the intention. Is "Pure" (not really) for the same instance. I'm trying to understand if for this use case the render impurity would make sense and not something that would be considered a violation. (like isHydrated use case)

No matter what I think helped removing one of the setState that where around for the blocker :)

Cheers

[brophdawg11]

isHydrated is changed in a useEffect - which is the correct way to perform side effects, and is how we're currently incrementing the blocker key. The issue React advises against is side effects in the render pass, which is happening with the useMemo incrementing approach.

[fernandojbf]

I was mentioning that isHydrated is used in components in a non pure way.

Indeed the change of it is done in a useEffect, but in terms of use does not make it pure since is not part of state or props xD