react-gauge-chart icon indicating copy to clipboard operation
react-gauge-chart copied to clipboard

Gauge re-initialized whenever parent re-renders (needle animates from 0)

Open RobRendell opened this issue 3 years ago • 12 comments

I don't know why this is a problem only I appear to be seeing, but when the parent of a GaugeChart component re-renders due to prop changes, the GaugeChart needle resets to 0 and then animates to its current value, causing it to oscillate wildly when there are frequent prop changes. This happens even if the props passed to GaugeChart don't change (the parent renders other components as well). I'm using useMemo to ensure the props to GaugeChart like arcsLength and colors don't change.

I've verified (using an onEffect hook) that the parent is only re-rendering, and not being unmounted and re-mounted.

I did a bit of debugging in the GaugeChart code, and I was able to see that this call to initChart() with no parameters inside the useLayoutEffect hook was being called every time the parent's props changed. The dependencies listed for the useLayoutEffect hook include the whole props object for the GaugeChart component - doesn't that mean that the callback passed to useLayoutEffect will be re-invoked every time the GaugeChart props change?

I was able to stop the behaviour in my application by rolling back to [email protected]. The introduction of the useLayoutEffect hook with props in its dependencies seems to have happened with merge request #74.

RobRendell avatar Aug 20 '21 06:08 RobRendell

@stevenhankin

Martin36 avatar Aug 21 '21 16:08 Martin36

I noticed the same and other issues. I'll be pushing a merge request that cleans this up a lot. The simple fix for this is to memoise the gauge props used by the useLayoutEffect like

const memoProps = React.useMemo(() => props, [JSON.stringify(props)])

And use memoProps instead of props. I would recommend pulling out just the create gauge props and memoising that instead of all the props though.

Also, the useDeepCompareMemo fn seems broken too. I believe line 10 should be if (!isDeepEquals(.... Currently it's updating the memoised deps if equal? https://github.com/Martin36/react-gauge-chart/blob/master/src/lib/GaugeChart/customHooks.js#L10

bjbrewster avatar Oct 06 '21 23:10 bjbrewster

Did you ever get round to doing a PR for this @bjbrewster? 🤞

rossdeane avatar Feb 17 '22 10:02 rossdeane

Hello,

I didn't create a PR but I did finish cleaning it up for the project I am on. I will commit to creating a PR within a few days. Please pester me again if I don't, lol!.

Cheers

On Thu, Feb 17, 2022 at 8:19 PM Ross @.***> wrote:

Did you ever get round to doing a PR for this @bjbrewster https://github.com/bjbrewster? 🤞

— Reply to this email directly, view it on GitHub https://github.com/Martin36/react-gauge-chart/issues/81#issuecomment-1042789852, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANJAYZYWIS25EEKEKOP5X3U3TDTXANCNFSM5CPV7TNQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

bjbrewster avatar Feb 17 '22 22:02 bjbrewster

@bjbrewster Is this merged ?

faizankhan1995 avatar May 09 '22 18:05 faizankhan1995

@bjbrewster any (good) news regarding the PR?

raymar avatar May 19 '22 11:05 raymar

I'm actively working on this and have been testing it lots on our website. I will push this in the next few days. Please bare with me. I've added some tests to make sure my refactoring doesn't break anything. I would appreciate a few people trying it out after I've pushed it. I will aim to push by the end of this weekend.

bjbrewster avatar May 19 '22 12:05 bjbrewster

I'm actively working on this and have been testing it lots on our website. I will push this in the next few days. Please bare with me. I've added some tests to make sure my refactoring doesn't break anything. I would appreciate a few people trying it out after I've pushed it. I will aim to push by the end of this weekend.

@bjbrewster let me know once you are done, I will do some tests.

raymar avatar May 25 '22 08:05 raymar

I'm guessing you're still doing tests and haven't made the PR? This single feature underwent more testing than Cyberpunk 2077 😂😂

ShayAxelrod avatar Nov 03 '22 07:11 ShayAxelrod

@bjbrewster Any chance you can create a pull request for your fixes? Is there anything we can do to assist?

inactivist avatar Dec 08 '22 20:12 inactivist

Hello. I'm very sorry life got in the way this last year. I have a week off next week for Xmas so I will push what I've done. It's almost a full rewrite. I tried to redo it in smaller commits but it took too long.

On Fri, 9 Dec 2022, 6:31 am Michael Curry, @.***> wrote:

@bjbrewster https://github.com/bjbrewster Any chance you can create a pull request for your fixes??

— Reply to this email directly, view it on GitHub https://github.com/Martin36/react-gauge-chart/issues/81#issuecomment-1343316995, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANJAY276RTWEPV46VDPPS3WMJAQNANCNFSM5CPV7TNQ . You are receiving this because you were mentioned.Message ID: @.***>

bjbrewster avatar Dec 20 '22 07:12 bjbrewster

Hello everyone, I have grown a great interest in this exceptional project and have made several modifications on my own. One of them includes a complete overhaul of the rendering process, which potentially resolves the issues you mentioned (and also #126, #44 (I'm planning to make some new types), #31, #27, #100, #57 and #122). Instead of submitting a pull request, I decided to create a new repository due to the disruptive changes and crazy experiments I'm still doing, feel free to check it out and test the modifications, let me know what u find.

You can find the repository here: https://github.com/antoniolago/react-gauge-component

Cheers to Martin36 and all collaborators for creating this great project, not easy to find a free and nice gauge component.

antoniolago avatar May 26 '23 04:05 antoniolago