Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-rendering plot when any state of the component changes #648

Closed
vaasu070 opened this issue Apr 22, 2021 · 53 comments
Closed

Re-rendering plot when any state of the component changes #648

vaasu070 opened this issue Apr 22, 2021 · 53 comments
Assignees

Comments

@vaasu070
Copy link

vaasu070 commented Apr 22, 2021

This behaviour was not observed with older version of chartjs. i.e version less than 3.
When ever the state of the component changes , entire plot re renders again as if it is rendering for the first time. sample code to this behaviour is here

@YakovlevCoded
Copy link
Collaborator

Yeah, thanks. i will work with it issue.

@YakovlevCoded
Copy link
Collaborator

@vaasu070 I have released a test package that solves this problem. Could I ask to try it out. And if everything is OK, I will release a minor version with an edit.

@vaasu070
Copy link
Author

@YakovlevCoded yeah now It is working as expected. Thanks for looking into it.

@vaasu070
Copy link
Author

vaasu070 commented Apr 24, 2021

@YakovlevCoded , I observed one more issue with test-react-chartjs-2 package, whenever data is updated, the chart fails to show the new data. It is always showing the initial data. This can be observed here . Hope this will be addressed soon. Thanks

@YakovlevCoded
Copy link
Collaborator

@vaasu070 Thank you! I will work on it.

@vaasu070 vaasu070 changed the title Re-rendering plot when the any state of the component changes Re-rendering plot when any state of the component changes Apr 29, 2021
@vaasu070
Copy link
Author

vaasu070 commented Apr 29, 2021

@YakovlevCoded A new version 3.0.3 is rolled out, But the above bug still exists in it here. If its not too early to ask, when it will be fixed and released? Its quite critical for out project Thanks.

@YakovlevCoded
Copy link
Collaborator

YakovlevCoded commented Apr 29, 2021

@vaasu070 If it's critical I'll try to finish it in the next couple of days. I'll let you know as soon as I'm done with this.

@vaasu070
Copy link
Author

@vaasu070 If it's critical I'll try to finish it in the next couple of days. I'll let you know as soon as I'm done with this.

Would really appreciate this!

@revroad-dev
Copy link

revroad-dev commented May 6, 2021

The re-rendering still seems to be an issue on current version 3.0.3. This was not an issue in chart.js v2.9.4

When the state changes the doughnut is redrawn/reanimated. Desired behavior is to rotate the doughnut without redrawing the graph.

export default function KChart() {
  const [rotation, setRotation] = useState(0);
  const onClick = () => {
    setRotation(rotation + 20);
  };
  const data = {
    labels: ['Red', 'Blue', 'Yellow', 'Green', 'Purple', 'Orange'],
    datasets: [
      {
        label: '# of Votes',
        data: [12, 19, 3, 5, 2, 3],
        backgroundColor: ['Red', 'Blue', 'Yellow', 'Green', 'Purple', 'Orange'],
        borderColor: ['Red', 'Blue', 'Yellow', 'Green', 'Purple', 'Orange'],
        borderWidth: 1,
      },
    ],
  };
  return <Doughnut type="doughnut" data={data} options={{ rotation, onClick }} />;
}

@revroad-dev
Copy link

The above happens on all chart types whether or not the state is passed to the component or not. (in other words, taking rotation out of the options objects still results in the same re-rendering behavior)

@ntamas
Copy link

ntamas commented May 7, 2021

I'm using the following patch as a band-aid until the issue is fixed properly. This probably does not cover all use-cases (it definitely doesn't handle datasets being removed from the chart), but it works for me with a simple bar chart that is refreshed every second:

diff --git a/node_modules/react-chartjs-2/dist/index.modern.js b/node_modules/react-chartjs-2/dist/index.modern.js
index f39264d..f8636d6 100644
--- a/node_modules/react-chartjs-2/dist/index.modern.js
+++ b/node_modules/react-chartjs-2/dist/index.modern.js
@@ -117,11 +117,14 @@ var ChartComponent = forwardRef(function (props, ref) {
     var _chart$config$data$da = chart.config.data.datasets,
         currentDataSets = _chart$config$data$da === void 0 ? [] : _chart$config$data$da;
     assign(chart.config.data, newChartData);
-    chart.config.data.datasets = newDataSets.map(function (newDataSet) {
+    newDataSets.forEach(function (newDataSet) {
       var currentDataSet = find(currentDataSets, function (d) {
         return d.label === newDataSet.label && d.type === newDataSet.type;
       });
-      if (!currentDataSet || !newDataSet.data) return newDataSet;
+      if (!currentDataSet) {
+        currentDataSet = {};
+        currentDataSets.push(currentDataSet);
+      }

       if (!currentDataSet.data) {
         currentDataSet.data = [];
@@ -130,7 +133,7 @@ var ChartComponent = forwardRef(function (props, ref) {
       }

       assign(currentDataSet.data, newDataSet.data);
-      return _extends({}, currentDataSet, newDataSet, {
+      assign(currentDataSet, newDataSet, {
         data: currentDataSet.data
       });
     });

@revroad-dev
Copy link

revroad-dev commented May 13, 2021

Hi @YakovlevCoded, thanks for all your hard work so far. Is this fix getting close to being released? Critical for our project.

@maidinhkhoa92
Copy link

I'm having same issue. Can I ask about release time ?

@vaasu070
Copy link
Author

Hi @YakovlevCoded any update on this?

@marcusround
Copy link

I have also run into this issue ( I posted about it here: #678 )
The test-react-chartjs-2 branch DID fix the issue.
However, there seems to be a separate issue with that branch, in that all my bars stopped drawing. It is because I was using data as a function: data: (canvas) => { return { /*...*/ } } so that I could use canvas patterns as a backgroundColor. If I change data to a normal object: data: { /*...*/ }, the bars draw fine, and the animation works.

@vaasu070
Copy link
Author

vaasu070 commented Jun 3, 2021

I have also run into this issue ( I posted about it here: #678 )
The test-react-chartjs-2 branch DID fix the issue.
However, there seems to be a separate issue with that branch, in that all my bars stopped drawing. It is because I was using data as a function: data: (canvas) => { return { /*...*/ } } so that I could use canvas patterns as a backgroundColor. If I change data to a normal object: data: { /*...*/ }, the bars draw fine, and the animation works.

To add to your observation on test-react-chartjs-2 , whenever data is updated, the chart fails to show the new data. It always shows initial data. This can be observed here

@marcusround
Copy link

To add to your observation on test-react-chartjs-2 , whenever data is updated, the chart fails to show the new data. It always shows initial data. This can be observed here

I am not sure what you mean exactly but I think its an issue with your code - see my working fork here

For my patterns issue, I have illustrated it here

@vaasu070
Copy link
Author

vaasu070 commented Jun 4, 2021

To add to your observation on test-react-chartjs-2 , whenever data is updated, the chart fails to show the new data. It always shows initial data. This can be observed here

I am not sure what you mean exactly but I think its an issue with your code - see my working fork here

For my patterns issue, I have illustrated it here

I was saying with test-react-chartjs-2 package in the code sample heres, initial data is {} and later data is changed to

{
  labels: [1, 2, 3, 4],
  datasets: [
    {
      data: [1, 2, 3, 4]
    }
  ]
}

But this is not reflected on the plot. with react-chartjs-2 it works perfectly fine.

@joey-carlisle4
Copy link

I have a similar issue and am awaiting a conclusion.

@marcusround
Copy link

I have a similar issue and am awaiting a conclusion.

For what its worth I was able to fix the issue (without introducing the patterns bug) by using the fix suggested by @ntamas in this thread.

@lucapollani
Copy link

lucapollani commented Jun 16, 2021

Encountered the same problem, it seems quite critical.
Is there a bugfix which will be released soon?

@joey-carlisle4
Copy link

I have a similar issue and am awaiting a conclusion.

For what its worth I was able to fix the issue (without introducing the patterns bug) by using the fix suggested by @ntamas in this thread.

I thought the fix was going to involve a custom build. Once I realized its just patching local node_modules, I realized its not as scary as it looks. Good fix for now!

@davidkipstar
Copy link

Hello,
I tried to apply the patch and also tried out test-react-chartjs-2 branch but with Bubble Chart, apparently it is not working with the patch and on the test branch I get no render at all. Did someone try this patch with Bubble Chart and succeded?

@maidinhkhoa92
Copy link

@vaasu070 I found how to render with new value.
you can combine test-react-chartjs-2 and useState
When fetching new data, loading is true. Loading state is false when you have new data

{loading ? (
                <Spinner />
              ) : (
                    <LineChart
                      params={params}
                      chartResponse={chartLineResponse}
                      isBoundary={isBoundary}
                      isZoom={isZoom}
                      averageValue={averageValue.value}
                      measurement={measurement.value}
                    />
              )}

@csmartinsfct
Copy link

csmartinsfct commented Jun 26, 2021

@vaasu070 I have released a test package that solves this problem. Could I ask to try it out. And if everything is OK, I will release a minor version with an edit.

the package worked fine for me with Doughnut and Line charts, thanks!

@mssrmaps
Copy link

I am having a similar issue, where a re-render due to state change on one chart component is causing a re-render on all charts (in their own components) under the same parent component to the parent of the one that had the state change.

This is occurring with hover, new data, etc. This did not occur on chart.js versions 2.9.4 and earlier. The resulting animations from the re-render are undesirable and I worry of related performance hits.

Is this a related issue to what is being described here?

@csmartinsfct
Copy link

I am having a similar issue, where a re-render due to state change on one chart component is causing a re-render on all charts (in their own components) under the same parent component to the parent of the one that had the state change.

This is occurring with hover, new data, etc. This did not occur on chart.js versions 2.9.4 and earlier. The resulting animations from the re-render are undesirable and I worry of related performance hits.

Is this a related issue to what is being described here?

Yes, try this package if you can and let people know if it works for you :)

@daviavmello
Copy link

daviavmello commented Jun 30, 2021

Very interesting solution by @ntamas. Subscribing to this post to know when this will be merged. Thank you for your hard work, @YakovlevCoded.

@nohaapav
Copy link

nohaapav commented Jul 2, 2021

Stacked bar chart is broken as well with test-package. Not rendering any data. With normal package works ok.

@dominicboston
Copy link

dominicboston commented Jul 7, 2021

Just came across this new bug that doesn't happen on the original package just on the test package. Not sure if this is my fault or not. Just posting incase anyone knows if this is a fixable issue. Just changed the datasets triggered by a data change. Issue only on test package.

Screenshot 2021-07-07 at 11 07 31

Screenshot 2021-07-07 at 11 19 15

@hardy12994
Copy link

This resolves my issue!

const LineChart = () => <Line data={data} options={options} />
const areEqual = (prevProps, nextProps) => true;

export default React.memo(LineChart, areEqual);

React Memo Document

@dominicboston
Copy link

This resolves my issue!

const LineChart = () => <Line data={data} options={options} />
const areEqual = (prevProps, nextProps) => true;

export default React.memo(LineChart, areEqual);

React Memo Document

This temp solution works for me too, be good to understand why this is happening on the test package and not the original.

Thanks

@daviavmello
Copy link

daviavmello commented Jul 19, 2021

I was able to fix this going off of @hardy12994's solution:

const handleClick = useCallback(() => {
}, []);
const memoizedChart = useMemo(DonutChart, [handleClick, data]);

@YassienW
Copy link

YassienW commented Jul 20, 2021

Experiencing the same issue, chart re-renders from scratch on both data changes and parent component re-renders. Seems to be fixed on test-react-chartjs-2
The memo fix seems to only fix part of the problem (unnecessary rerenders but not data changes)

I am wondering why a relatively critical issue like this hasn't been merged yet.

Edit: this happens with newer versions of chart.js as well

@strblr
Copy link

strblr commented Jul 20, 2021

Same problem here. I'm going back to version 2 for the time being.

@ae0n
Copy link

ae0n commented Jul 28, 2021

test-react-chartjs-2 is fixing this problem. Please include those changes in a new minor version

@enjineerMan
Copy link

enjineerMan commented Aug 11, 2021

I'm using chartjs-plugin-zoom for zooming and panning. On zoom or pan events (onZoomComplete and onPanComplete in chartStyle), a function called getVisibleValues is triggered that calls a setState function passed through props. However, calling this function resets the chart back to its default position, which makes panning or zooming impossible. I've tried both test-react-chartjs-2 and @ntamas ' fix, but the issue persists. Sandbox for illustration.

const SingleChart = ({ chosenDevice, variable, setVisible }) => {
  const [chartData, setChartData] = useState({
    labels: [],
    datasets: [
      {
        label: variable,
        data: [],
        fill: false,
        backgroundColor: "rgb(0,0,0)",
        borderColor: "rgba(0,0,0, 0.8)",
        color: "rgb(0,0,0)",
        pointRadius: 4,
        yAxisID: "y",
      },
    ],
  });
  const [apiData, setApiData] = useState({});
  const processEnv = process.env;

  function getVisibleValues({ chart }) {
    setVisible(chart.scales.x.ticks); //calling callback setState function
  }
  const chartStyle = {
    options: {
      animation: false,
      maintainAspectRatio: true,
      responsive: true,
      plugins: {
        zoom: {
          zoom: {
            wheel: {
              enabled: true,
              modifierKey: "shift",
            },
            pinch: {
              enabled: true,
            },
            enabled: true,
            drag: true,
            mode: "x",
            onZoomComplete: getVisibleValues,
          },
          pan: {
            enabled: true,
            mode: "x",
            speed: 2,
            onPanComplete: getVisibleValues,
          },
          mode: "xy",
        },
        legend: {
          display: false,
        },
      },

      scales: {
        y: {
          type: "linear",
          display: "true",
          position: "left",
          grid: {
            drawBorder: true,
            color: "#000000",
          },
          ticks: {
            beginAtZero: true,
            color: "#000000",
          },
        },

        x: {
          max: 9,
          grid: {
            drawBorder: true,
            color: "#00000",
          },
          ticks: {
            beginAtZero: false,
            color: "#000000",
          },
        },
      },
    },
  };

// code here removed for decluttering, gets chartData from an API

  return (
    <div>
      <Line
        data={chartData}
        options={variable != "GEN_ON" ? chartStyle.options : genStyle.options}
        width={20}
        height={10}
      />
    </div>
  );
};

@MaxboDev
Copy link

Hey, is there any update on this issue? Its quite a major problem for us and I'm wondering whether we'll have to switch to using chart.js directly before we go live. Thanks

@PCPbiscuit
Copy link

@M4sterShake Same here.

@mssrmaps
Copy link

@PCPbiscuit @M4sterShake
me too

@MaxboDev
Copy link

@PCPbiscuit @mssrmaps I've just tried upgrading to 3.0.5 and it fixed the issue for me. If you guys aren't on this version give it a go. Hope it works!

@MaxboDev
Copy link

MaxboDev commented Oct 2, 2021

Just to help clarify things for anyone reading this thread, it looks like this issue is fixed in 3.0.5. @YakovlevCoded it can probably be closed unless anyone has an issue after upgrading to 3.0.5.

@dangreen
Copy link
Collaborator

react-chartjs-2@v3.2.0 was released with rerendering improvements. So I'm closing this issue, but feel free to open new ones if any problems appear.

@Elius94
Copy link

Elius94 commented May 2, 2022

For me the bug is still alive :( same as previous post says

@dangreen
Copy link
Collaborator

dangreen commented May 2, 2022

@Elius94 Hi. Please open new issue with repoduction.

@larinius
Copy link

larinius commented Dec 8, 2022

Have exact same issue - re-rendering of donut chart when any app state changed. It looks very ugly and distracting.
"react-chartjs-2": "^5.0.1",

@dangreen
Copy link
Collaborator

dangreen commented Dec 8, 2022

@larinius Hi. Please provide issue reproduction. You can fok codesandbox from examples.

@awhite9
Copy link

awhite9 commented Jun 15, 2023

@larinius - this might be a little late - but I just spent a week wracking my brain over performance issues rendering pages with multiple charts with very large datasets constantly re-rendering every time the parent stat changed - slowing down my entire page.... React memo solved my issue - basically made my child components with the chartjs components no re render every time the parent does, only when its down state or props change

@wayneschuller
Copy link

@larinius - React memo solved my issue - basically made my child components with the chartjs components no re render every time the parent does, only when its down state or props change

Can you post sample code with useMemo and react-chartjs-2 chart improvements?

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

No branches or pull requests