Discussion: chart.update architecture

[anajavi]

Opening new issue to discuss architecture change

@whawker wrote:

Going completely off topic, I've been wondering if useModifiedProps is a mistake, currently when props change we figure out which ones have changed, and commit them as updates to the wrapped Highcharts chart.

I wonder instead if we should be utilising the oneToOne option of chart.update so that we can sure that the state in React representation of the chart, is the same as the internal Highcharts representation. However this might be undermining one of the fundamental principles of this project in the first place, to avoid destroying the chart completely between React re-renders)

[anajavi]

This could be done without the oneToOne option too.

All the updates could be centralised to the BaseChart so that individual components such as Series declare their desired state to the BaseChart. BaseChart then does deep compare of previous chart state to current state, and performs changes to the chart.

For example:

1. Series component is mounted
2. Series component calls BaseChart via context: baseChart.updateSeries(props)
3. Legend is mounted
4. Legend calls BaseChart via context: baseChart.updateLegend(props)
5. BaseChart compares previous chartState with the changes caused by child components
6. BaseChart calls relevant chart.update, axis.update etc

Centralising changes also helps to keep track of axes when highstock decides to recycle the whole object (Navigator if I remember correctly).

[whawker]

I like the idea of updateAxis and updateSeries, but not keen on updateLegend as I'd like to avoid having lots of methods like updateNavigator, updateRangeSelector etc in baseChart. I fear it would mean baseChart needs to know about every Highcharts feature.

Perhaps for more generic updates we could baseChart.update('legend', props) instead?

The currently architecture whilst having flaws, gives us a nice separation of concerns (in my opinion), so I would be keen on keeping that principle where possible.

Would it be worth using immer internally, to maintain the overall chart state?