chore(react): upgrade React typings to use a more explicit children prop declaration

[virtuoushub]

React 18 removed implicit children from the React.FunctionComponent type.

This PR improves our use of typings to make more explicit when a functional component has a children prop, and as a bonus will allow us to more easily allow migrate to React 18 (#4992).

---

See also:

• https://solverfox.dev/writing/no-implicit-children/
• [RFC]: Upgrading to React 18 for Redwood #5881

[Tobbe]

I'm sorry PC, but I'm not a big fan of PropsWithChildren. The React team basically added that helper type to make it easier to write codemods for React 17 -> 18 upgrades. I'd much rather we just use React.FC when we upgrade to React 18, and explicitly include children in the Props interface.

Dan Abramov himself agrees with all of this too: https://stackoverflow.com/a/71809927/88106

[virtuoushub]

I'm sorry PC, but I'm not a big fan of PropsWithChildren. The React team basically added that helper type to make it easier to write codemods for React 17 -> 18 upgrades. I'd much rather we just use React.FC when we upgrade to React 18, and explicitly include children in the Props interface.

Dan Abramov himself agrees with all of this too: https://stackoverflow.com/a/71809927/88106

No worries, tomato / 🍅; happy to change this to explicitly include children in the respective prop types/interfaces.

---

As a small aside, curious what the drawback is of using a helper type?

[Tobbe]

As a small aside, curious what the drawback is of using a helper type?

First, to be clear, for sure it's mostly a personal preference. And because of that, if the rest of the team rather use PropsWithChildren then so be it 🙂

But, some reasons are:

• A helper type is one more indirection.
  • One more step you have to jump through to get to the full picture of the type.
  • You can't just look at the Props interface to see all props for the component
• It's one more type you have to learn
• Using children directly demystifies it, showing people it's just another prop.

[virtuoushub]

...

one more indirection.

...

Using children directly demystifies it, showing people it's just another prop.

...

tbh - I also felt this way when implementing, something about the ergonomics of the wrapper was off; so fwiw having an explicit children prop is also my personal preference.

For whatever reason at the time I implemented (months ago now), it seemed easier just to drop in the helper interface 🤷🏻

[Tobbe]

This is probably for another PR, but we should also try to decide on React.FunctionComponent vs React.FC. (I prefer FC)

[virtuoushub]

This is probably for another PR, but we should also try to decide on React.FunctionComponent vs React.FC. (I prefer FC)

Agreed although I am much more in the FunctionComponent camp, as FC provides little semantic value.

🙃 - I have had this on my radar for a bit. I already have a separate PR cut for it, but since that change is pretty related to this one; wanted to get this one merged first.

---

[Tobbe]

as FC provides little semantic value

In general I agree with this sentiment. But FunctionComponent is just so verbose 😬
This being RW with our generators that might not matter so much though 🤔
Do you have any sense of what's more common in the wider React community? What would other developers be more used to?

We should discuss this with the team before deciding.

[pantheredeye]

as FC provides little semantic value

In general I agree with this sentiment. But FunctionComponent is just so verbose 😬 This being RW with our generators that might not matter so much though 🤔 Do you have any sense of what's more common in the wider React community? What would other developers be more used to?

We should discuss this with the team before deciding.

It may be slightly more welcoming to new users to see it written out.

FWIW there is a discussion here that edges close to this question: typescript-cheatsheets/react#190. That also took me to the cheatsheet link: https://react-typescript-cheatsheet.netlify.app/docs/basic/getting-started/function_components/ (oh, gotta click the dropdown on "Why is FC discouraged...")

@IThinkThatsKirby mentioned trying to be more verbose in the codebase vs less.

[Tobbe]

typescript-cheatsheet says "However, the general consensus today is that React.FunctionComponent (or the shorthand React.FC) is discouraged." That was (maybe) true with the old React 17 types, but I really don't agree anymore with the new React 18 types.

But that cheatsheet has been cited as one of the reasons we didn't do React.FunctionComponent before.

[pantheredeye]

typescript-cheatsheet says "However, the general consensus today is that React.FunctionComponent (or the shorthand React.FC) is discouraged." That was (maybe) true with the old React 17 types, but I really don't agree anymore with the new React 18 types.

But that cheatsheet has been cited as one of the reasons we didn't do React.FunctionComponent before.

I almost made the caveat that I was not trying to point to that link for the discourage mention. They note FC as the shorthand, but throughout the note they use FunctionComponent.

[Tobbe]

I shouldn't get so worked up about this, but I've had this exact argument at work. Where someone wants to use FunctionComponent for the explicitness and newbie friendliness, and I prefer FC for the convenience. That same guy also wants us to use <React.Fragment> instead of <> for the same reasons, and I'm equally (if not more) against that 🙂 But I will obviously get behind whatever the team decides 🙂

[IThinkThatsKirby]

I shouldn't get so worked up about this, but I've had this exact argument at work. Where someone wants to use FunctionComponent for the explicitness and newbie friendliness, and I prefer FC for the convenience. That same guy also wants us to use <React.Fragment> instead of <> for the same reasons, and I'm equally (if not more) against that 🙂 But I will obviously get behind whatever the team decides 🙂

TLDR:TDLR:TDLR: 🩳✋= 💯 if 📜 🔥

TLDR:TLDR: <React.Fragment> is insane. Its akin to never using an apostrophes or contraction. If used in great frequency then short hand assisted with good documentation, otherwise spell it out to be kind to others.

That's my take, I'll leave the cringe below I spent to much time over explaining and don't want to feel like my effort was wasted.

TLDR: <React.Fragment> is to hand holdy. Anyone with a will to solve problems can figure that out really fast. VS Code yells at you about it immediately. But those that know more should be consulted about if we need <React.FunctionalComponent> spelled out. Its really more of a question as to how well the documentation aids in understanding less used shorthand.

My thoughts on this are pretty simple. That which stands the longest gets the most detail. deletes 7 paragraphs of philosophical we stand on giants , age of fire, gates open come on in blah blah blah commie manifesto, phalanx formation, insert lord of the rings Return of the king quote here. Ok so my personal cringe aside. I realize the debate is more of a where is the "good enough" place to draw the line. <React.Fragment> I agree is not necessary for problem solving. Using the tool React one finds out very fast at least with VS Code that you need to wrap things in a <> </>. So do new users or contributors of RedwoodJS need to have React.FunctionComponent spelled out for faster onboarding? In struggle do we learn and innovate, but 1 letter variables, cryptic variable names, bad documentation, and confusing design choices all lead to burn out.

[Tobbe]

@IThinkThatsKirby Thanks for sharing. I enjoyed reading all of it 🙂