Definitelytyped: [@types/recharts] RechartsFunction has no arguments

Created on 19 Oct 2017  路  9Comments  路  Source: DefinitelyTyped/DefinitelyTyped

  • [x] I tried using the @types/recharts package and had problems.
  • [x] I tried using the latest stable version of tsc. https://www.npmjs.com/package/typescript
  • [x] I have a question that is inappropriate for StackOverflow. (Please ask any appropriate questions there).
  • [x] [Mention](https://github.com/blog/821-mention-somebody-they-re-notified) the authors (see Definitions by: in index.d.ts) so they can respond.

    • Authors: @mthmulders @rapmue @royxue

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

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;

All 9 comments

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.

https://github.com/recharts/recharts/blob/38417b6960a05cadeb5e5f58578da207044b9c4c/src/chart/generateCategoricalChart.js#L1016

    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.

  1. @wojtekZajac suggested (data: any, index: number, event: MouseEvent) => void
  2. @chelmertz suggested (mouse: ?, e: ?) => void (using ? here to indicate an unknown type)
  3. Looking at the BarChart with custom event sample, I think it's something like (data: any, index: number) => void

Is 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;

Was this page helpful?
0 / 5 - 0 ratings