Switch to mount dispatcher after use() when needed

[sophiebits]

When resuming a suspended render, there may be more Hooks to be called that weren't seen the previous time through. Make sure to switch to the mount dispatcher when calling use() if the next Hook call should be treated as a mount.

Fixes #25964.

[sophiebits]

I had a little trouble wrapping my head around whether it's necessary to do something with HooksDispatcherOnMountWithHookTypesInDEV here but my DebugValue test passes so ¯\_(ツ)_/¯?

[acdlite]

That dispatcher is related to the hooks mismatch warning, not useDebugValue. But I think I recall the last time I looked at that code, it was due to a weird legacy mode only quirk related to React.lazy. We should try to clean it up (separately). Still, probably the worst thing that happens is we sometimes don't fire a warning where we should.

The other weird case is the InvalidNestedHooksDispatcher(s). If we want to match production exactly then we have to account for those too. That's where you call a hook inside of e.g. useMemo. But that's considered undefined behavior so also not super critical.

So I think we can merge as-is. Neither of those are as important as fixing the bug.

[sophiebits]

Yeah sorry – HooksDispatcherOnMountWithHookTypesInDEV is because non-stateful hooks (like useDebugValue) can trigger the mismatch warning. Some of the tests in ReactHooks-test.internal using useContext or useDebugValue fail without it. But I couldn't get it to happen here with the test I added.

Can you elaborate on what needs to be done with the InvalidNestedHooks ones? From a quick glance I would think we're already OK there and it will warn as expected.

[acdlite]

The finally block when you exit a useMemo call sets the dispatcher back to the previous one.

react/packages/react-reconciler/src/ReactFiberHooks.js

Lines 2972 to 2976 in 96cdeaf

	ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV;
	try {
	return mountMemo(create, deps);
	} finally {
	ReactCurrentDispatcher.current = prevDispatcher;

So for example, if you were to call use inside useMemo (which triggers a warning in dev), I think what would happen is that the dispatcher would get overridden twice.

Although now that I type that out, maybe it just works because the useMemo computation would have been reused from last time. So the invalid hook call wouldn't happen.

[sophiebits]

Meh. I agree it's undefined behavior and you get the first warning so I don't really care.

[sophiebits]

Hmm although now is the dev and prod behavior different…

[react-sizebot]

Comparing: ca2cf31...5f120bc

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js	+0.02%	154.46 kB	154.48 kB	+0.05%	48.75 kB	48.77 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	+0.02%	156.45 kB	156.47 kB	+0.04%	49.41 kB	49.43 kB
facebook-www/ReactDOM-prod.classic.js	+0.03%	530.70 kB	530.88 kB	=	94.57 kB	94.58 kB
facebook-www/ReactDOM-prod.modern.js	+0.03%	514.63 kB	514.80 kB	+0.01%	92.12 kB	92.13 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 5f120bc

[acdlite]

Maybe we could turn nextWorkInProgressHook into a module level variable and read from there. To avoid a tiny bit of duplication. And/or could split use into separate mount and update implementations. But since this is a fairly specialized branch it's fine this way too.

[acdlite]

🐐🐐🐐 thanks Sophie!