React: Enforce defaultProps for props that are not required

[jendowns]

Summary

Please consider requiring defaultProps definition for all props that aren't required.

Specifically, via react/require-default-props eslint rule.

Justification

Without defaultProps, a prop is set to undefined. Sometimes this isn't a problem, but sometimes it is, as in #6076 (actually not exactly relevant in this case)

Additional examples of why defaults for non-required props are important can be found in the react/require-default-props docs.

[joshblack]

Hey there @jendowns! 👋

Sorry about the bug, agreed this rule would have caught it 😞

When trying to enable it on our project, it does seem like this causes a lot of noise for props that most likely don't need a default value. How would you recommend dealing with this?

I think the most obvious example in our codebase is className, another one is children

[jendowns]

Hey @joshblack! My instinct here is to proceed to add defaults for className and children as well-- it would be a LOT to add, but the good thing here is that you can probably just set them all to undefined. So at least it would be some copy and pasting 😅 But yeah it would be a pretty big PR...

[joshblack]

@jendowns it seems like that would have '' as the default for className, what would you recommend as a default for children?

[jendowns]

For the default for children I would recommend undefined (in @carbon/ibm-security it looks like there are 16-ish components with children: undefined for defaults and 14-ish with children: null as defaults 😅 )

[joshblack]

Ugh, I'm definitely a little hesitant to add defaults in this way, but it does seem like the size bug has been pesky. I'm not sure why exactly the defaults aren't being added in and will check with the team.

Let me see if there are some other examples of bugs that we could track under this rule to see if we can justify bringing it in. I'm also going to go through and update our eslint config to reflect better rules for writing components. (Let me know if you have any other rules that you'd recommend!)

Until then, I think we'll keep the rule set stable for now (I'll definitely add in the recommended rules per-plugin) and as I find bugs that would have been fixed by this I'm going to add them to the issue. I think 3 or more would be a good number to justify this change 👍

[jendowns]

Thanks @joshblack -- the size issue that I opened today appears to be a problem with our versioning (we need to update to get a fix), so I updated the desc above.

I understand the hesitation & the need to proceed carefully, as it is a LOT to change, but I think it would be very helpful in the long-run to add this rule. And it would encourage good practices around component props.

[joshblack]

Going to close this out since it's been a while, miss you at IBM @jendowns! Hope you're having fun at your current gig 🥳