Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead. at {Component}

[marco-digennaro]

Using a library that include recharts v2.6.2 i get the following error in the console.

Curve: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead. at Curve

the charts are rendered normally thou.

[ckifer]

Thanks, noted! Unfortunately moving away from defaultProps causes unintended bugs due to the way recharts works. But this is known and we're trying to get away from them where we can.

[cdimitroulas]

FYI this also happens for the Tooltip component

[ckifer]

Reference for why this is an issue https://github.com/recharts/recharts/issues/3438#issuecomment-1467419256

[ckifer]

@akamfoad all of our functional refactors are going to have this issue. We may have to prepared to handle soon as I'm going to release 2.7. Let me know if you have any thoughts

[akamfoad]

@ckifer I was always feeling bad about the defaultProps but in my refactors I kept them just in case and to reduce the number of changes.

But I think the sooner we embrace JS default params the better, I see many teams are going in this direction, especially those who use TypeScript, I see that is quite the default practice.

This is not going to break semver if we do it in v2, because we just change how we receive the default values, internally, for the users of the library everything will be the same and the warning goes away. What do you think?

[ckifer]

@akamfoad I totally agree, but we still have the cloneElement issue linked above (https://github.com/recharts/recharts/issues/3438#issuecomment-1467419256).

I believe all of the components we have refactored besides Tooltip are safe from that issue so we should be able to move them over

[ckifer]

I believe this list holds those that we cannot touch without fear of breaking things

[akamfoad]

So only those components will cause issue if defaultProps is removed, that another component/function needs to access them, including cloneElement, before the component is initialized.

I'm guessing there is a lot of them, especially those from the list in generateCategoricalChart.

Which means they in fact cause breaking change, when migrated to default params. So we might need to hold off here and revert the Tooltip refactor, that is an easy one.

The warning is shown only for Functional Components with defaultProps, but the default behavior may changes in the future for how this affects components (if I understood the RFC correctly). Which means, its probably for the best to prepare for v3.

[akamfoad]

Btw @ckifer

I think its probably for the best, this might encourage us to have major version (not very soon tho), where we internally do not rely on defaultProps, or cloneElement, instead we use normal JS default params, and renderProps (which is not my personal favorite), or Context API.

This way, the children can both render and subscribe to context's provided by the charts (or a an HoC) parent and render, clutter-free

[ckifer]

Thanks for the thoughts @akamfoad! Yeah I agree - I've been shying away from 3.x because it's a lot of work and there's a lot of breaking changes we need to merge in and document but there hasn't really been any meaningful progress made towards it.

But yes we definitely need to 100% rid ourselves of the cloneElement approach, it's the cause of a lot of issues (but it's also some of the magic of recharts which is why it's tough to remove or at least tough to test once you remove it).

[jocarrd]

Same problem, any solution?

[ckifer]

No solution yet but we're aware of it, thanks

[shohan-pherones]

I also need the solution asap, do your job devs.

[ckifer]

There are no devs that work on this project as a job. Please feel free to contribute.

[zwhitchcox]

Just leaving this here:

const error = console.error;
console.error = (...args: any) => {
  if (/defaultProps/.test(args[0])) return;
  error(...args);
};

[ooanishoo]

zwhitchcox

Hi, is this a temporary fix that we can implement for now?

[zwhitchcox]

@ooanishoo This will just hide the warning, but there's not really anything else you can do until it's fixed upstream anyway.

[wuahi]

Having the same issue. Is there an update on where this might be fixed?

[evanlong0803]

I was just about to ask a question, but I guess I won't. Everyone knows 🤣

[ckifer]

No update really besides some noodling in my head to try to pick a path forwards. If I work on anything at all next it will be this, I just can't make time for it right now as other (life) priorities prevail.

[ckifer]

Risky elements are defined (by me at least 😅 ) as any that are referenced by a call to findAllByType findChildByType and those in the map in generateCategoricalChart (see issue linked above - https://github.com/recharts/recharts/issues/3438#issuecomment-1467419256). This comes out to:

1. Cell
2. Brush
3. Tooltip
4. Legend
5. ErrorBar
6. XAxis
7. YAxis
8. ZAxis
9. PolarAngleAxis
10. PolarRadialAxis
11. RadialBar
12. Label
13. LabelList
14. ReferenceDot
15. ReferenceLine
16. ReferenceArea
17. CartesianGrid
18. PolarGrid
19. Customized
20. Any graphical child - Bar, Area, Line, Scatter, Funnel, Pie, Radar, RadialBar

All other components outside of these should be able to be refactored to use default params with relative certainty they won't break things. This makes this issue actionable and means that Text, Curve and any others not on this list can be moved over any time. @nikolasrieble @akamfoad for vis.

If anyone wants to take the changes or the change for one component let me know.

[ckifer]

Text refactored in #3670. Curve is open for contribution as well as any others that may currently have issue

[ckifer]

Tooltip is one of the affected components that gets funky. findChildByType is called for Tooltip multiple times in different places. Some use props that don't have a default set, others do.

Safe references:

• https://github.com/recharts/recharts/blob/38253c9806aab8099c2182297a90f7e04706205f/src/chart/Treemap.tsx#L359-L361 - only checking for existence in react tree
• https://github.com/recharts/recharts/blob/38253c9806aab8099c2182297a90f7e04706205f/src/chart/Treemap.tsx#L381-L383 - existence check
• https://github.com/recharts/recharts/blob/38253c9806aab8099c2182297a90f7e04706205f/src/chart/generateCategoricalChart.tsx#L1214 - reference to shared prop which is not defined in defaultProps

Unsafe references:

• https://github.com/recharts/recharts/blob/38253c9806aab8099c2182297a90f7e04706205f/src/chart/Sankey.tsx#L466-L484 - use of trigger which is set in default props
• https://github.com/recharts/recharts/blob/38253c9806aab8099c2182297a90f7e04706205f/src/chart/generateCategoricalChart.tsx#L1352-L1356 - reference to trigger which is set in defaultProps. This one wouldn't break though as the value its referencing is not the default
• https://github.com/recharts/recharts/blob/38253c9806aab8099c2182297a90f7e04706205f/src/chart/generateCategoricalChart.tsx#L2055 - reference to existence as well as trigger (but same as above, not a check for the default value)
• https://github.com/recharts/recharts/blob/38253c9806aab8099c2182297a90f7e04706205f/src/chart/Sankey.tsx#L489-L494 - reference to trigger
• https://github.com/recharts/recharts/blob/38253c9806aab8099c2182297a90f7e04706205f/src/chart/Sankey.tsx#L512-L514 - reference to trigger

Unsure:

• https://github.com/recharts/recharts/blob/38253c9806aab8099c2182297a90f7e04706205f/src/chart/generateCategoricalChart.tsx#L1931-L1945 - this one calls React.cloneElement on the found Tooltip with some new props... I'm not sure if react merges these with defaultProps or not (I think shallow merge?), I would assume so and therefore unsafe
• https://github.com/recharts/recharts/blob/38253c9806aab8099c2182297a90f7e04706205f/src/chart/Treemap.tsx#L672 - same as above

[ckifer]

https://github.com/recharts/recharts/pull/3671 fixes Curve

Tooltip... is still an issue as seen above. Please yell out any other components that throw the error. Looks like Label and LabelList, PolarGrid, Cross, Rectangle, Sector, Symbols, Trapezoid at least

Looks like Tooltip, Label, LabelList are the risks, the rest are free to refactor

[ckifer]

https://github.com/recharts/recharts/pull/3681 fixes Trapezoid and Symbols

This is about the speed I can go right now with some spurts here and there, open to contributions as always 🙏🏼

[akamfoad]

3685 fixes Cross

3686 fixes Rectangle

[ckifer]

All of the easy components have been refactored to safely use default params - this leaves Tooltip, Label, LabelList without a great solution.

we can

1. revert back to classes - not preferred
2. store a map of default params to component type and merge user params with default params where findByType is used - semi-hacky and 1000% temporary but probably preferred
3. ?

[akamfoad]

Wouldn't 2 make maintenance even more complicated?

[ckifer]

Yeah I guess it depends on how we did it. Maybe it's worth just reverting back to classes until the macro problem is addressed

[akamfoad]

Initially this effort was to make the implementation simpler so we can easily do changes, but if this makes it more complex I'd say reverting them is the better option now.

[ckifer]

yeah, as much as I hate to do that I agree. we have the refactors in git history to go back to so it isn't like they are much effort

[nikolasrieble]

Good call @akamfoad! Thank you. I believe we must focus our effort on the large problems. Lets revert the initial refactoring instead of further adding complexity. Lets come up with an attack plan for generateCategoricalChart 😅

[akamfoad]

A while back I put some efforts into that, the result was pathetic, it fights back hard😅

You can see my progress here: https://github.com/akamfoad/recharts/tree/akamfoad/migrate-generateCategoricalChart

EDIT: opened a draft PR for this work in case someone is interested in the progress, https://github.com/recharts/recharts/pull/3709

[nikolasrieble]

@akamfoad I believe we should first refactor the component - extract unit testable functions and add unit tests. The function must be as small as possible, and only a high level combination of calls to smaller - tested - helpers. Only then can we reasonably refactor.

[akamfoad]

@nikolasrieble that is definitely a better approach, I did bite the whole thing at once😂

I'm gonna take another look soon.

[ckifer]

I was thinking at looking at React Router and how they prevent certain components from being rendered and within others (Route can't be rendered outside of Routes). I know maybe eventually we want to rid ourselves of that type of setup, but it might be worth a peruse to get this same type of functionality.

Maybe we should move this to a separate discussion

[ckifer]

Label and LabelList have always been functions :/ - this leaves us to revert Tooltip, but idk if I really want to re-write Label and LabelList as a class...

They only each have one defaultProp though - lets see if we can't test these specifically to make sure they don't break things

[evanlong0803]

Refactor it using radix-ui. 🤗

[ckifer]

Refactor it using radix-ui. 🤗

@evanlong0926 that seems pretty unrelated 😅

Either way @akamfoad @nikolasrieble Label and LabelList seem safe to me - they do not directly reference props after findAllByType, findChildByType - they call cloneElement which correctly invokes the component and causes default params to be set.

See here and here

Also the true danger comes from stuff like this - calling one of these and then directly accessing element.props.somethingThatWasDefaulted

[ckifer]

These should all be resolved in 2.8.0. Please help me check/bug bash. Thank you all for your patience <3

[ckifer]

Going to close this for now, will re-open if there are any issues

[alex-grover]

I'm still seeing this error logged for ReferenceDot in 2.8.0. With a quick search, it looks like defaultProps is also still used in X/Y/ZAxis, ReferenceLine/Area, and ErrorBar.

[ckifer]

Oof, these ones were like this previous to us beginning to refactor components to functions and have a lot of sketchy references that will break if we remove them like this one https://github.com/recharts/recharts/blob/16f66c3ff656d02524aa8c31b1f2d35b15be94f5/src/util/ChartUtils.ts#L1179-L1185

Definitely more hesitant to take action on the axes but the Reference components and ErrorBar should be able to be refactored safely...

@alex-grover does the error get logged if you use X/Y/ZAxis?

I still can't reproduce these errors so it is tough to test - if anyone can get them to log in a sandbox that would be incredible

[alex-grover]

I'm not seeing the error logged with the Axis components.

Had some issues getting a sandbox set up, but this minimal example produces the error for me in case that works for you: https://github.com/alex-grover/recharts-default-props-console-error

[ckifer]

Thank you!

[JacobWeisenburger]

Warning: ReferenceLine: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.

I'm still having this problem on recharts: 2.10.3

any workarounds?

[jocarrd]

@JacobWeisenburger same error

[hyperscientist]

fyi, ReferenceLine still has the issue

[ckifer]

Heard that this is still an issue. These ones weren't priority as they have been this way for some time and the other components were not. This issue will remain open until ReferenceLine, ReferenceArea, ReferenceDot are refactored

[arobert93]

I've updated to the latest version and it seems that I now get this error for XAxis and YAxis.