meteor / react-packages

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

react-meteor-data@2.5.1: can't use hooks in withTracker #363

Open wolasss opened 1 year ago

wolasss commented 1 year ago

It seems that in 2.5.1 you can't use hooks inside withTracker anymore. I referenced this issue in https://github.com/meteor/react-packages/issues/354#issuecomment-1133885651 and now I am creating a separate issue for it.

Please take a look in the reproduction repository:

https://github.com/wolasss/meteor-react-data-2.5.1-bug

I used meteor react boilerplate, I simply added useContext in withTracker to demonstrate the issue:

export default withTracker(()=>{
    const contextValue = useContext(TestContext);

    return {
        links: LinksCollection.find().fetch(),
        contextValue
    }
})(Info);

Well, it does not have to be useContext but any other hook. It throws an error:

Error: Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:
1. You might have mismatching versions of React and the renderer (such as React DOM)
2. You might be breaking the Rules of Hooks
3. You might have more than one copy of React in the same app
See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem.
    at Object.throwInvalidHookError (modules.js?hash=680ff16881b1d15688855d95d47029972b45b8a0:18867:13)
    at useContext (modules.js?hash=680ff16881b1d15688855d95d47029972b45b8a0:2628:21)
    at Info.jsx:22:26
    at withTracker.tsx:19:15
    at useTracker.ts:69:18
    at Computation._compute (tracker.js:308:7)
    at Computation._recompute (tracker.js:324:16)
    at Tracker._runFlush (tracker.js:495:14)
    at onGlobalMessage (meteor.js?hash=b9ec8cf25b6fc794ae6b825f30e06c3c35c50e7c:520:23)

If you switch to v2.4.0, the problem does not exist (branch v2.4.0 in reproduction repo)

wolasss commented 1 year ago

This seems to be a regression introduced in https://github.com/meteor/react-packages/commit/384da2f5d1b130cc9b41030b95d4b33961c6e0df . In the app where I have a bunch of hooks inside withTracker the whole app goes to infinite loop of rendering (both react v17 and v18). Maybe it is related to the "an attempt to retain the "side-effect" we are creating during first render" @CaptainN ?

CaptainN commented 1 year ago

It's not really a regression, because you really shouldn't be using other hooks within useTracker's computation handler (whether used with the hook or hoc). That was never supported, and I'm surprised you haven't run in to problems sooner.

Meteor really should not support this in useTracker (and family), and we should even consider throwing a warning when such use is detected.

Instead, you should compose your hooks within another hook (or hoc), so that they work side by side, and not nested.

CaptainN commented 1 year ago

You can make your own hoc out of useTracker, and then compose your hooks something like this:

const withMahStuff = (Component) => (props) => {
  const links = useTracker(() => {
    return LinksCollection.find().fetch()
  }, [])
  const contextValue = useContext(TestContext)
  const newProps = {
    ...props,
    links,
    contextValue
  }
  return <Component {...newProps} />
}

export default withMahStuff(Info)

Check out more examples from the old useTracker post on medium.

StorytellerCZ commented 1 year ago

Let's deprecate withTracker and throw a warning.

henriquealbert commented 1 year ago

I agree with you, @StorytellerCZ - or at least- give some guidance on the warning.