Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove all uses of React.FC in our TypeScript code #1553

Closed
humphd opened this issue Jan 18, 2021 · 2 comments · Fixed by #1667
Closed

Remove all uses of React.FC in our TypeScript code #1553

humphd opened this issue Jan 18, 2021 · 2 comments · Fixed by #1667
Assignees
Labels
area: front-end area: nextjs Nextjs related issues developer experience Helping the Developer Experience type: bug Something isn't working
Milestone

Comments

@humphd
Copy link
Contributor

humphd commented Jan 18, 2021

What happened:

We have a mix of different approaches to defining our React components in the next.js TypeScript. In some places we use React.FC for typing, and in others we don't.

Let's remove all instances of React.FC.

What should have happened:

According to facebook/create-react-app#8177, there are some issues with always using FC, and we'd probably be better served by removing it. I also think this will be a win for new React devs who don't know TS very well, and encounter FC for the first time in our code.

If you're reviewing new code, ask the developer to remove FC. For any that slips in, let's do a sweep and remove it all later on.

See also discussion in #1531 (comment).

@humphd humphd added type: bug Something isn't working area: front-end developer experience Helping the Developer Experience area: nextjs Nextjs related issues labels Jan 18, 2021
@HyperTHD
Copy link
Contributor

HyperTHD commented Jan 26, 2021

Update: As of January 25th, 2021, only the Banner component uses FC. Once DynamicBannerImage is ported over and we start work on the Banner component, this will change. We can close this issue and work on #1557 after the Banner component is ported over.

1 FC Component Left

@HyperTHD
Copy link
Contributor

HyperTHD commented Feb 6, 2021

#1667 is the PR that addresses the last component using React.FC, which is the banner component. Once that's taken cared of, this issue can be closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: front-end area: nextjs Nextjs related issues developer experience Helping the Developer Experience type: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants