@types/recharts package and had problems.Definitions by: in index.d.ts) so they can respond.The RechartsFunction which is used in all components onClick callbacks has no arguments, whereas it should have data, index and event arguments.
I guess it should be something like this:
RechartsFunction = (data: any, index: number, event: MouseEvent) => void
I'm not sure about this one. We've had earlier discussions about RechartsFunction (see #19756). It is very generic, and I don't know if adding data, index and event to it is valid in every place where it is used. @rapmue, how do you feel about it?
@mthmulders As the OP said, the callback arguments for onClick are missing. Instead of using a generic RechartsFunction we can add a OnClickFunction with the corresponding arguments like the formatters.
But we have to make sure the signature of this callback is persistent over all components from recharts.
It seems like the handleClick event is pretty generic.
handleClick = (e) => {
const { onClick } = this.props;
if (_.isFunction(onClick)) {
const mouse = this.getMouseInfo(e);
onClick(mouse, e);
}
};
All of the handleClick events should at least be generic enough to type, perhaps it is enough to switch out only the onClick functions for now.
@mthmulders @rapmue @royxue
Any update on this? Can we at least change it to something like type RechartsFunction = (...args:any[]) => any; if you're not going to properly type each handler?
This makes the entire @types/recharts lib completely unusable to anyone who needs an onClick handler... 馃槩
That sounds like a nice approach to me, @sarink. It would make it possible to implement onClick handlers and access data, index and event parameters as needed.
Although a more thorough approach by re-defining the type for onClick handlers would be good for me, too. I don't have a strong preference for one solution or the other.
@mthmulders Writing specialized onClick definitions for all the different types of charts would definitely be preferable, but in the meantime the any solution would at least let us poor click-event-needers compile the app.
@sarink, I could take a shot at it.
I'm not sure about the type signature of OnClickHandler, though.
(data: any, index: number, event: MouseEvent) => void(mouse: ?, e: ?) => void (using ? here to indicate an unknown type)(data: any, index: number) => voidIs there someone who can shed light on this? Or should we draw the conclusion that there is no generic OnClickHandler because the onClick prop is interpreted in too many different ways?
Maybe @xile611, who has written most of recharts, can be fooled into participating in this discussion :) (Not sure if he gets this highlight, so I will poke him through email as well, hoping not to come off as a freeloader :/ )
@mthmulders I think each type of chart is going to be different. We'll likely end up with barChartOnClickHandler, pieChartOnClickHandler, ...etc.
Until all of these are fully specified and implemented, I think adding this one line would satisfy all conditions for everyone, so it should be implemented asap (the rest can be added piecemeal later):
export type RechartsFunction = (...args:any[]) => any;
Most helpful comment
@mthmulders I think each type of chart is going to be different. We'll likely end up with
barChartOnClickHandler, pieChartOnClickHandler, ...etc.Until all of these are fully specified and implemented, I think adding this one line would satisfy all conditions for everyone, so it should be implemented asap (the rest can be added piecemeal later):
export type RechartsFunction = (...args:any[]) => any;