fix: premature cleanup in strict mode and improved performance

[welkinwong]

When strict mode is enabled (development mode only), useEffect may run 1 to 2 times. Due to the outer layer throwing a promise, the multiple executions of useEffect in this scenario can lead to premature cleanup of subscriptions and cachedSubscription (i.e., before the component unmounts). Therefore, it is necessary to check the timeout to ensure that subscriptions and cachedSubscription are only cleared after the component has unmounted.

[welkinwong]

Supplementary Description: Premature cleanup of subscriptions and cachedSubscription can lead to infinite re-mounting of components in strict mode (especially when used in conjunction with useFind, causing repeated execution of useSubscribe). This occurs because prematurely clearing cachedSubscription results in re-throwing a Promise, which suspends the component and creates a loop.

[StorytellerCZ]

This would explain some of the issues I ran into! Looking forward to a patch release with this!

[StorytellerCZ]

Also very much appreciate the lodash removal.

[nachocodoner]

I understand the problem you describe and the code makes sense. But, is there any chance to provide tests capturing the problem you describe?

I’m hesitant to merge changes in these critical packages without reliable evidence that we fully understand the issue and the fix. Since we’ve just restored tests in this repository, it’s a good opportunity to ensure they’re up to date and accurately cover the problem.

[welkinwong]

I understand the problem you describe and the code makes sense. But, is there any chance to provide tests capturing the problem you describe?

I’m hesitant to merge changes in these critical packages without reliable evidence that we fully understand the issue and the fix. Since we’ve just restored tests in this repository, it’s a good opportunity to ensure they’re up to date and accurately cover the problem.

@nachocodoner Your argument is perfectly valid, so I've added two test cases to cover these changes. ：）

[welkinwong]

This test fails on the legacy code but passes in the current PR.

[nachocodoner]

I didn't realize that the changes here are also included, #430

Closing then in favor of #435

[nachocodoner]

This change is available in meteor add react-meteor-data@3.0.5-beta.0. Please, verify it works as expected.

[welkinwong]

This change is available in meteor add react-meteor-data@3.0.5-beta.0. Please, verify it works as expected.

Verified - normal operation 👏