YAxis does not rerender when passed a new min/max

[llaver]

What?

Currently the YAxis component does not rerender when new min/max props are provided. This is unfortunate for developers who would prefer variable axes.

Why?

The YAxis component always returns false in the shouldComponentUpdate react lifecycle method. According to @pjm17971 this is "because d3 is controlling the DOM below this component, the component doesn't re-render in the same way because that would take control from d3. Instead [react-timeseries-charts] uses the props change to force d3 to re-create / alter the DOM that it controls." This means that elsewhere in the project, there should be a check for a change in min/max and that is not happening.

When?

Any time after initial load when the component is passed new min/max props. May happen with other props as well, but I have not experimented with any others.

How?

1. Create a new chart with a YAxis who's min and max props are set by parent props.
2. Do something that causes one or more of those props to change.
3. New props are passed to YAxis.
4. Axes do not update.

This is even more obvious when the axis prop you are passing to LineChart is the same prop that you are passing to YAxis. The LineChart will rerender (points/line are plotted), and will properly scale to the updated axis prop, but since the axes don't also update, the scale of the plotted lines is immediately inaccurate.

Solution

The issue will be solved when:

• Wherever the chart is handling updated props, it also checks if min or max have been updated and rerenders/recreates the chart axes.

[llaver]

After investigation and experimentation, it seems allowing shouldComponentUpdate to return true when appropriate does not rip control from d3 and rerendering the component works as you would expect it to. From what I can tell there have been no adverse affects and everything still seems to work properly.

It would be good to get some further testing, so I've made a pull request #348 for others to test out and give feedback.

[pjm17971]

Are you sure about this issue, because we change the min/max of our axes all the time?

[llaver]

I'll try to get a sample repo up with the issue present, but I am 100% updating min/max and definitely not getting any rerender/change. The change in the PR solved the problem in my project without causing any other issues (pretty advanced/built out usage). My fear with this change is with what you said:

the component doesn't re-render in the same way because that would take control from d3

However, I discussed this with my team lead and he believes if that were the case, the component wouldn't be able to render properly in the first place (I'm not sure his full reasoning behind this, but he seemed confident).

After every attempt to solve the issue changing nothing but project code failed, and the PR changes worked so simply, I decided to push it for others to try out/analyze/point out flaws.

I would love to see something that DOES break from this so I can better understand why it could be an issue.

[llaver]

I has been about a month since I made the change in PR #348 to my forked version used in my project and I have had no issues so far.

[dirkdevriendt]

FYI We are dynamically adding charts, adapting Y Axes etc as a user selects different data series and ran into the same issue, resulting in a "select(...).transition is not a function" error (just like https://github.com/esnet/react-timeseries-charts/issues/231 ?) and a page crash.

Since we needed a quick solution, we implemented a workaround by including a random string in the axes' names and corresponding chart-axis references whenever the user selection changes. This seems to work just fine for our use case.