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

[ui] Eliminate React.FC #17446

Merged
merged 1 commit into from
Nov 6, 2023
Merged

[ui] Eliminate React.FC #17446

merged 1 commit into from
Nov 6, 2023

Conversation

hellendag
Copy link
Member

Summary & Motivation

Remove React.FC from JS packages, some by hand but mostly using https://github.com/gndelia/codemod-replace-react-fc-typescript.

In some places, I went in and manually tidied things to prevent huge whitespace shifts, especially around React.memo.

How I Tested These Changes

TS, lint, jest.

@hellendag
Copy link
Member Author

hellendag commented Oct 27, 2023

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@github-actions
Copy link

github-actions bot commented Oct 27, 2023

Deploy preview for dagit-storybook ready!

✅ Preview
https://dagit-storybook-5zus9dd06-elementl.vercel.app
https://dish-fc-cleanup.components-storybook.dagster-docs.io

Built with commit 8bf8831.
This pull request is being automatically deployed with vercel-action

@github-actions
Copy link

github-actions bot commented Oct 27, 2023

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-geflzys4n-elementl.vercel.app
https://dish-fc-cleanup.core-storybook.dagster-docs.io

Built with commit 8bf8831.
This pull request is being automatically deployed with vercel-action

Comment on lines +89 to +101
'@typescript-eslint/ban-types': [
'error',
{
types: {
'React.FC':
'Useless and has some drawbacks, see https://github.com/facebook/create-react-app/pull/8177',
'React.FunctionComponent':
'Useless and has some drawbacks, see https://github.com/facebook/create-react-app/pull/8177',
'React.FunctionalComponent':
'Preact specific, useless and has some drawbacks, see https://github.com/facebook/create-react-app/pull/8177',
},
},
],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lint guardrail

Copy link
Collaborator

@bengotow bengotow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good! Thanks for linking out to the docs, I had heard that this was not preferable anymore (was it ever?) but reading the full list of reasons in that issue is great. RIP React.FC!

@hellendag hellendag merged commit 684a01c into master Nov 6, 2023
5 checks passed
@hellendag hellendag deleted the dish/fc-cleanup branch November 6, 2023 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants