sunburst-chart icon indicating copy to clipboard operation
sunburst-chart copied to clipboard

Chart getting duplicated for every useEffect call in react app

Open jeyk333 opened this issue 2 years ago • 7 comments

In my react, I am calling the chart method inside useEffect with data and also passing the data variable in the dependency array. Initially, its rendering properly. But, whenever, the data get updated, instead of the updating the chart, its creating new chart on every useEffect call. How can I solve this chart duplicating and instead of it, I need the chart to get updated.

import React, { useEffect, useRef, useState } from 'react'
import Sunburst from 'sunburst-chart'
import * as d3 from 'd3'

const SunburstChart = ({ data }) => {
  const color = d3.scaleOrdinal(d3.schemePaired)

  useEffect(() => {
      Sunburst()
        .data(data)
        .width(740)
        .height(740)
        .sort((a, b) => b.value - a.value)
        .color((d) => color(d.name))
        .excludeRoot(true)(document.getElementById('chart-sunburst'))
    }
  }, [data])

  return (
    <div id="chart-container">
      <div id="chart-sunburst"></div>
    </div>
  )
}

export default SunburstChart

Screenshot 2022-11-09 at 10 39 30

jeyk333 avatar Nov 07 '22 14:11 jeyk333

@vasturiano Please let me know, how to fix this one

jeyk333 avatar Nov 09 '22 05:11 jeyk333

@jeyk333 thanks for reaching out.

This seems to be more of a question around React rather than this Sunburst component. I can't advise specifically what should be the React pattern for you to use here, maybe a React forum would be more appropriate for that, but reinstantiating a Sunburst chart everytime the data changes (via useEffect) is certainly not right. The chart should be instantiated only once within a single component's lifecycle.

You can also use react-kapsule as it does all the wrapping for you out of the box.

vasturiano avatar Nov 09 '22 10:11 vasturiano

Ok, let me check with react-kapsule library

jeyk333 avatar Nov 09 '22 13:11 jeyk333

@vasturiano I have tried using React-kapsule. But, the similar issue occurs here too.

import React, { useEffect, useRef, useState } from 'react'
import Sunburst from 'sunburst-chart'
import * as d3 from 'd3'

import fromKapsule from 'react-kapsule'

const ReactSunburst = fromKapsule(Sunburst, {
  methodNames: ['focusNode', 'onClick'],
})

const SunburstChart = ({ data, loading }) => {
  const chartRef = useRef(null)
  const color = d3.scaleOrdinal(d3.schemePaired)
  const [newData, setNewData] = useState(data)

  return (
    <div id="chart-sunburst" ref={chartRef}>
      <ReactSunburst
        width={740}
        sort={(a, b) => b.value - a.value}
        color={(d) => color(d.name)}
        label={(d) => (d.value ? `${d.name} - ${d.value}` : d.name)}
        height={740}
        excludeRoot
        data={newData}
      />
    </div>
  )
}

export default SunburstChart

Screenshot 2022-11-09 at 21 36 10

Whenever, I tried to interact with the chart, this console error occurs and chart is not zooming

jeyk333 avatar Nov 09 '22 16:11 jeyk333

@jeyk would you mind making a reproducing simple example on https://codesandbox.io/ ?

vasturiano avatar Nov 09 '22 22:11 vasturiano

sure

jeyk333 avatar Nov 10 '22 06:11 jeyk333

In React v18 the duplicate is expected when using Strict Mode in development. It won't show up in production or if you remove Strict Mode.

Replace: <React.StrictMode> <App /> </React.StrictMode> With: <> <App /> </> and you'll see the duplicate no longer available.

SeaShackleton avatar Aug 12 '23 15:08 SeaShackleton

For those of us who do not have the option of removing StrictMode, there is a better solution. Please see this issue explanation for more on why this happens and how to implement specifically in a React app.

You do not need to use Kapsule (and I would recommend against using it) Essentially what @jeyk333 needs to do here is add a cleanup function to his useEffect of

return () => {
      if (chartRef.current?.children[0]) {
        chartRef.current.removeChild(chartRef.current.children[0]);
      }
    };

and that should do the trick.

@vasturiano This issue should probably now be marked as closed.

coreymunn3 avatar Jun 05 '24 21:06 coreymunn3