meteor / react-packages

Meteor packages for a great React developer experience
http://guide.meteor.com/react.html
Other
571 stars 157 forks source link

Invalid data returned if deps did not change. #329

Closed DavidSichau closed 3 years ago

DavidSichau commented 3 years ago

We run into a problem with useTrackerWithDeps when we used not a subscription inside the reactive function.

Observation

The use case is that we sometimes want to conditional subscribe if certain props are set in a component. And then we simply skip the subscription inside the reactive function and return some data. We then observed that useTracker did not return the intended data but instead always undefined.

Some Example of our Code that triggered the problem:

usePage = () => {
  const data = useTrackerWithDeps(
    () => {
    if (props.page) {
      const subscription = Meteor.subscribe('page', pageId)
      const page = Pages.findOne({ _id: pageId })
      return {
        page,
        isLoading: !subscription.ready()
      }
   } else {
      return { blub: 1 }
   }
  }, [props.page])
 // do something with data
 // with old useTrackerWithDeps data was here undefined if props.page was not set if usePage was rendered more than once.
 return data;
}

Explanation

We observed then the behavior that only correct data from useTrackerWithDeps was returned during the first render. On subsequent renders always undefined was returned. As the useEffect of useTrackerWithDeps was not executed as the deps did not change. However, the data were not available anymore in the state.

Solution

To solve this issue we decided to store the current data from the reactiveFn inside a ref instead of the state, as then on subsequent renders the initial value is still available, even if useEffect is not executed.

CaptainN commented 3 years ago

This is very similar to the solution in #321 - that one has tests with it. Can you check out that PR and see if that solves your issue?

DavidSichau commented 3 years ago

321 Fix this problem also. Therefore closing it in favor of #321.