meteor / react-packages

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

react-meteor-data #316

Closed DenysOf closed 3 years ago

DenysOf commented 3 years ago

Hi, react-meteor-data@=2.2.X is introducing an error in the App which worked on react-meteor-data@=2.1.X component affected: useTracker() The App react version: "react": "^17.0.1",

Warning: Maximum update depth exceeded. This can happen when a component calls setState inside useEffect, but useEffect either doesn't have a dependency array, or one of the dependencies changes on every render.

Downgrading to react-meteor-data@=2.1.4 works.

Screenshot 2021-02-03 at 17 30 01
CaptainN commented 3 years ago

Can you confirm that this is still a problem in 2.2.2?

Can you also provide a reduction?

DenysOf commented 3 years ago

@CaptainN, sorry, looks like still a problem on 2.2.2

Can you please clarify on reduction?

CaptainN commented 3 years ago

A reduction is a reproduction of the problem from larger code base, which clearly demonstrates the problem without the cruft.

I suspect the problem you are having is pretty much what that error is describing - that something in your deps array is changing on every render. But it might help me work out a mitigation strategy if I had a reproduction. It might also help me point out some alternative ways to work with the hook in a way which doesn't trigger the error.

A reduction/reproduction will also help me determine whether there is a problem in the hook that needs to be addressed.

filipenevola commented 3 years ago

Hi @DenysOf, you could read more about reproductions here

DenysOf commented 3 years ago

Hi @DenysOf, you could read more about reproductions here

Yes, I'm aware of re-production, just didn't thinking its a typo reduction .

DenysOf commented 3 years ago

Sorry, for delay, have been crazy busy with other stuff. Please see below:

const useBStats = () => useTracker(() => { const subscription = Meteor.subscribe("BlockRaw"); const bStats = BlockRaw.findOne({}, {sort: {blockHeight: -1}}); const meteorStatus = Meteor.status(); passed = false; return { bStats: bStats, isLoading: !subscription.ready(), meteorStatus: meteorStatus, }; }, []); const useStats = () => useTracker(() => { const subscription = Meteor.subscribe("ValidatorStats"); const stats = ValidatorStats.findOne({}, {sort: {blockHeight: -1}}); const meteorStatus = Meteor.status(); passed = false; return { stats: stats, isLoading: !subscription.ready(), meteorStatus: meteorStatus, }; }, []); const useDelegators = (account_ids, numRec) => useTracker(() => { const subscription = Meteor.subscribe("Delegators", account_ids); const delegators = Delegators.find({}, {limit: numRec}).fetch(); const meteorStatus = Meteor.status(); passed = false; return { delegators: delegators, isDelegatorsLoading: !subscription.ready(), meteorStatus: meteorStatus, }; }, [account_ids]);

I have more then one useTracker hook in one component, is this a problem?

DenysOf commented 3 years ago

@CaptainN @filipenevola have made an effort and amalgamated all into one useStats function (many subscriptions in one useTracer block), but it didn't help.

CaptainN commented 3 years ago

A reduction is part of a reproduction. The distinction is not that important. Sorry if that was a little bit of esoteric jargon.

CaptainN commented 3 years ago

If you could provide a little more context (the use of those hooks) that might help. For example, depending on how useDelegators is called, this code may not work, because the account_ids array may be getting a fresh reference on every render.

DenysOf commented 3 years ago

If you could provide a little more context (the use of those hooks) that might help. For example, depending on how useDelegators is called, this code may not work, because the account_ids array may be getting a fresh reference on every render.

Thanks for clarifying reduction always great to learn something new :)

Sure, see below,

const {delegators, isDelegatorsLoading} = useDelegators(a, stateCtx.config.lastEpochs);

      let t = [];
      for (const d of delegators) {
        t.push(d.pool)
      }
....

hope this helps

CaptainN commented 3 years ago
 let t = [];
 for (const d of delegators) {
   t.push(d.pool)
 }

So that's it exactly - that array instance is getting refreshed on every render (because it's a fresh reference each render), which is pretty much exactly what the error is telling you is happening:

Warning: Maximum update depth exceeded. This can happen when a component calls setState inside useEffect, but useEffect either doesn't have a dependency array, or one of the dependencies changes on every render.

This issue has come up a lot in my teams with useEffect and useTracker - enough that I'm going to make a change to the useTracker docs, and recommend that deps not be used by default. They are optional with useTracker anyway. Supplying deps should be thought of as an optimization step, and not a standard practice with useTracker.

The specific way deps optimize useTracker is worth understanding too - it only really helps with renders after ~mount~ the render is committed, by pulling the computation out of the render (react) fiber (out from that single execution frame). That's not always going to be faster than just running your Meteor work right in the render.

To be clear - for your case, removing the deps array is the fastest/easiest way to solve your problem. If you still want to use deps, you'll need to find a way to cache that array reference (maybe useMemo, but that's not guaranteed, maybe using useRef, but that's difficult to maintain.)

DenysOf commented 3 years ago

Many thanks for the detailed research. I have removed the deps and its now working with react-meteor-data 2.2.2

Had to apply a workaround, as one of the deps, is changed on user input and should re-render and refresh computation with different parameter const subscription = Meteor.subscribe("Delegators", account_ids); Working now until I find a better solution. account_ids was in the dep. array, causing re-render.