[BarChart] Broken types for ActiveShape

[klassm]

• I have searched the issues of this repository and believe that this is not a duplicate.

Reproduction link

https://github.com/klassm/recharts-bar-chart-repro

Steps to reproduce

• Downgrade to recharts 2.8.0 -> compiles
• Upgrade to recharts 2.9.9 -> broken with multiple issues

What is expected?

• The name property can be used without casting (as string)
• The IData property, so the interface for the data, can be used within the Shape component (which was possible beforehand and still is, it is just prohibited by typing)

What is actually happening?

For the name:

Type  BarProps  is not assignable to type  Omit<SVGProps<SVGPathElement>, "radius"> 
Types of property  name  are incompatible.
Type  string | number | undefined  is not assignable to type  string | undefined

For the rest of the properties - they are just not available. Looking at the code, the ActiveShape interface just always uses BarProps and ignores the type of the data - which is also why typescript does not allow to have it.

Environment	Info
Recharts	v2.9.0
React	18.2.28
System	MacOS
Browser	Chrome

[akamfoad]

This change have been introduced by #3803, the signature of shape and others changed

-  shape?: ReactElement<SVGElement> | ((props: any) => ReactElement<SVGElement>);
+  shape?: ActiveShape<BarProps, SVGPathElement>;

So before the props where of type any I think, that is why you could do that, I understand this might be a breaking change. CC @nicholasgcoles @andrewangelle

You could work around it with render prop and make the (prop: any) => <CustomeShape />

[ckifer]

@andrewangelle FYI

[klassm]

Thanks for the response. Yes, casting it to any works - so there are workarounds; it is just - they shouldn't be necessary (especially casting to any -.-)

[akamfoad]

@klassm

Indeed, sorry if that landed poorly, I meant so you're not blocked until we fix this issue.

We respect the semver semantics and we had no idea this breaking change was introduced.

Thanks for reporting, hopefully we'll release a minor/patch fix to revert this back to the pre-breaking-change interface. Though I cannot promise about ETA since we all work on recharts on our own time, and contributions are always welcome.

Thanks again for the bug report.

[ckifer]

+1 to the above

Agreed @klassm - just as a quick explanation of why that type narrowed - recharts code is... not that great (and was originally JS) and so we began some cleanup work recently including making the type definitions more correct than they were, but mostly for things that are internal only as to not break anyone.

Code Review can only guard so much from issues like this one and occasionally they a get in - we'll make a fix in a patch for sure

[andrewangelle]

My apologies guys. Looking at this now, I should have known that given the previous type definition of ((props: any) => ReactElement<SVGElement>) then changing that any to anything else would be a breaking change. I'll have a PR up today to fix this.

[ckifer]

No worries, thanks @andrewangelle!

[klassm]

Thank you so much for taking care! I really appreciate you fixing this in your free time. Thanks!

[ckifer]

@klassm released in 2.9.1 - I pulled down your reproduction and built just to make sure it was fixed. All good! Closing this