Fix #3492: throw warning when use class component and suspense

[guochengcheng04]

Fix #3492

• unit test all passed.
• performance all passed.

[changeset-bot]

🦋 Changeset detected

Latest commit: af5313d

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

This PR includes changesets to release 1 package

Name	Type
mobx-react	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

[guochengcheng04]

@mweststrate hi bro, do you have any ideas about this PR?

[urugator]

See the comment: #3492 (comment)

[urugator]

Any reason not to import these utils from mobx-react-lite?

[urugator]

Note, it's a slightly artifical, but you can this[reactionTrackingRefSymbol] = React.createRef()

[guochengcheng04]

Note, it's a slightly artifical, but you can this[reactionTrackingRefSymbol] = React.createRef()

Yes, but I don't think it's necessary. When a function component uses ref, because it can't get the component instance, the class component can directly cache this to achieve the same effect

[guochengcheng04]

Any reason not to import these utils from mobx-react-lite?

If you think this is more reasonable, I can modify it here. Initially, a separate file was created for decoupling between different packages

[urugator]

Yes, mobx-react already depends on mobx-react-lite, it more or less only adds class support.

[guochengcheng04]

i will do it

[urugator]

Move at the end of createReaction, otherwise you would have to clear this[reactionTrack] whenever reactiveRender[mobxAdminProperty] is cleared.
EDIT: Taking it back as you have to do it anyway, because the existence of this[reactionTrack] is used as a check whether the reaction was disposed.

[urugator]

In order for this to happen, you have to unset this[reactionTrack] in componentWillUnmount, like here

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

Lines 103 to 104 in f0e066f

	reactionTrackingRef.current!.reaction.dispose()
	reactionTrackingRef.current = null

And also this[reactionTrack] must be unset when reaction is disposed in tracking utils, is it the case?

[urugator]

Btw, I didn't want to get into it initially, but for completeness, it can also be done without tracking utils in the following fashion:

// timeout could be used as well
const queueMicrotask = window.queueMicrotask ?? (microtask) => Promise.resolve().then(microtask)

// in render
queueMicrotask(() => { 
  if (!mounted) {
     disposeReaction()
  }
})

// in componentDidMount
mounted = true
if (reactionDisposed) {
  recreateReaction() // can be done lazily in render
  forceUpdate()
} else if (reactionNotified) { // state changed in beetwen
  forceUpdate()
}

This is because componenDidMount is quaranteed to run sychronously (before the microtask).
The same can be done with functional components, but we would have to use useLayoutEffect instead of useEffect - could have negative impact on concurrent features, dunno.

[guochengcheng04]

@urugator all change have done, check please, thank you

[guochengcheng04]

@urugator Can you help me approve and publish? If need me to do anything please tell me

[guochengcheng04]

@urugator Can you help me approve and publish? If need me to do anything please tell me

@mweststrate @kubk hi, can we merge and publish to finish this pr?