vasturiano / sunburst-chart

A sunburst interactive chart web component for visualizing hierarchical data
https://vasturiano.github.io/sunburst-chart/example/flare/
MIT License
292 stars 91 forks source link

Chart getting duplicated for every useEffect call in react app #100

Closed jeyk333 closed 5 months ago

jeyk333 commented 2 years ago

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 commented 2 years ago

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

vasturiano commented 2 years ago

@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.

jeyk333 commented 2 years ago

Ok, let me check with react-kapsule library

jeyk333 commented 2 years ago

@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

vasturiano commented 2 years ago

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

jeyk333 commented 2 years ago

sure

SeaShackleton commented 1 year ago

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> With: <>

</>` and you'll see the duplicate no longer available.

coreymunn3 commented 5 months ago

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.