[mobx-react] Maximum update depth exceeded error since mobx-react@8

[jeffijoe]

Intended outcome:

There should be no Maximum update depth exceeded error

Actual outcome:

React throws a Maximum update depth exceeded error.

How to reproduce the issue:

I created a reproduction repository (see src/MobX.jsx) - it includes both mobx and downshift, this is because downshift is the root cause however the way mobx-react behaves after version 8 (with the change to useSyncExternalStore) makes me believe it's worth investigating on the mobx-react side.

You'll be able to see for yourself by downgrading to mobx-react@7 in that repo and running the test - it will show 2 in the counter instead of 53 (at which point the error is thrown which stops the loop).

I know the issue template states to avoid speculation but I've spent way too long investigating what's going on to not not mention it 😅 What seems to be happening is the useSyncExternalStore will "push" another work item on the internal React sync queue (for batching) and because every reaction will make mobx-react's implementation return a new symbol in getSnapshot, if React triggers an effect which subsequently triggers a MobX reaction, it will keep pushing to that queue for each iteration.

mobx/packages/mobx-react-lite/src/useObserver.ts

Lines 49 to 72 in 13a74fe

	subscribe(onStoreChange: () => void) {
	// Do NOT access admRef here!
	observerFinalizationRegistry.unregister(adm)
	adm.onStoreChange = onStoreChange
	if (!adm.reaction) {
	// We've lost our reaction and therefore all subscriptions, occurs when:
	// 1. Timer based finalization registry disposed reaction before component mounted.
	// 2. React "re-mounts" same component without calling render in between (typically <StrictMode>).
	// We have to recreate reaction and schedule re-render to recreate subscriptions,
	// even if state did not change.
	createReaction(adm)
	// `onStoreChange` won't force update if subsequent `getSnapshot` returns same value.
	// So we make sure that is not the case
	adm.stateVersion = Symbol()
	}
	return () => {
	// Do NOT access admRef here!
	adm.onStoreChange = null
	adm.reaction?.dispose()
	adm.reaction = null
	}
	},
	getSnapshot() {

New work pushed to queue

As I mentioned, the root cause is a different library, but what prompted me to look into this was because in my real application, the browser tap freezes when this happens as the error actually loops infinitely there. I couldn't reproduce that here though. Additionally, when using plain React useState (as seen in Basic.jsx), it does not result in the error.

I have also opened an issue with downshift.

Versions

• mobx@6.12.3
• mobx-react@9.1.1
• react@18.2.0
• react-dom@18.2.0