Skip to content

Invalid data returned if deps did not change.#329

Closed
DavidSichau wants to merge 1 commit intometeor:masterfrom
DavidSichau:master
Closed

Invalid data returned if deps did not change.#329
DavidSichau wants to merge 1 commit intometeor:masterfrom
DavidSichau:master

Conversation

@DavidSichau
Copy link
Copy Markdown

@DavidSichau DavidSichau commented Apr 29, 2021

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
Copy link
Copy Markdown
Contributor

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
Copy link
Copy Markdown
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants