Skip to content

Commit

Permalink
[ui] Eliminate React.FC (#17446)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
hellendag authored Nov 6, 2023
1 parent 5e852ff commit 684a01c
Show file tree
Hide file tree
Showing 274 changed files with 1,632 additions and 1,053 deletions.
14 changes: 9 additions & 5 deletions js_modules/dagster-ui/packages/app-oss/src/NUX/CommunityNux.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export const CommunityNux = () => {
// Wait 1 second before trying to show Nux
const TIMEOUT = 1000;

const CommunityNuxImpl: React.FC<{dismiss: () => void}> = ({dismiss}) => {
const CommunityNuxImpl = ({dismiss}: {dismiss: () => void}) => {
const [shouldShowNux, setShouldShowNux] = React.useState(false);
React.useEffect(() => {
const timeout = setTimeout(() => {
Expand Down Expand Up @@ -77,10 +77,12 @@ const CommunityNuxImpl: React.FC<{dismiss: () => void}> = ({dismiss}) => {
);
};

const Form: React.FC<{
interface FormProps {
dismiss: () => void;
submit: (email: string, newsletter: boolean) => void;
}> = ({dismiss, submit}) => {
}

const Form = ({dismiss, submit}: FormProps) => {
const [email, setEmail] = React.useState('');
const [newsletter, setNewsLetter] = React.useState(false);
const validEmail = isEmail(email);
Expand Down Expand Up @@ -169,11 +171,13 @@ const Form: React.FC<{
);
};

const RecaptchaIFrame: React.FC<{
interface RecaptchaIFrameProps {
newsletter: boolean;
email: string;
dismiss: () => void;
}> = ({dismiss, newsletter, email}) => {
}

const RecaptchaIFrame = ({dismiss, newsletter, email}: RecaptchaIFrameProps) => {
const [iframeLoaded, setIframeLoaded] = React.useState(false);
const [width, setWidth] = React.useState(680);
const [height, setHeight] = React.useState(462);
Expand Down
13 changes: 13 additions & 0 deletions js_modules/dagster-ui/packages/eslint-config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,19 @@ module.exports = {
'react/prefer-stateless-function': 'error',
'react/prop-types': 'off',
'react/display-name': 'off',
'@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',
},
},
],
'@typescript-eslint/no-unused-vars': [
'error',
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ interface Props {
onClose?: () => void;
}

export const Alert: React.FC<Props> = (props) => {
export const Alert = (props: Props) => {
const {intent, title, description, onClose} = props;

const {backgroundColor, borderColor, icon, iconColor, textColor} = React.useMemo(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ interface IconProps {
fillColor: string;
}

const StarIcon: React.FC<IconProps> = ({checked, indeterminate, fillColor, disabled}) => (
const StarIcon = ({checked, indeterminate, fillColor, disabled}: IconProps) => (
<svg width="24px" height="24px" viewBox="-3 -3 24 24">
<path
className="interaction-focus-outline"
Expand Down Expand Up @@ -65,7 +65,7 @@ const StarIcon: React.FC<IconProps> = ({checked, indeterminate, fillColor, disab
</svg>
);

const SwitchIcon: React.FC<IconProps> = ({checked, indeterminate, fillColor, disabled}) => (
const SwitchIcon = ({checked, indeterminate, fillColor, disabled}: IconProps) => (
<svg width="36px" height="24px" viewBox="-3 -3 42 28">
<defs>
<linearGradient x1="50%" y1="0%" x2="50%" y2="100%" id="innerShadow">
Expand Down Expand Up @@ -103,7 +103,7 @@ const SwitchIcon: React.FC<IconProps> = ({checked, indeterminate, fillColor, dis
</svg>
);

const CheckIcon: React.FC<IconProps> = ({checked, indeterminate, fillColor, disabled}) => (
const CheckIcon = ({checked, indeterminate, fillColor, disabled}: IconProps) => (
<svg width="24px" height="24px" viewBox="-3 -3 24 24">
<path
d="M16,0 C17.1,0 18,0.9 18,2 L18,2 L18,16 C18,17.1 17.1,18 16,18 L16,18 L2,18 C0.9,18 0,17.1 0,16 L0,16 L0,2 C0,0.9 0.9,0 2,0 L2,0 Z"
Expand Down Expand Up @@ -178,9 +178,16 @@ const Base = ({
...rest
}: Props) => {
const uid = useRef(id || uniqueId());
const Component: React.FC<IconProps> = {star: StarIcon, check: CheckIcon, switch: SwitchIcon}[
format
];
const Component = React.useMemo(() => {
switch (format) {
case 'star':
return StarIcon;
case 'check':
return CheckIcon;
case 'switch':
return SwitchIcon;
}
}, [format]);

return (
<label htmlFor={uid.current} className={className} onClick={onClick}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ interface Props {
isOpen: boolean;
}

export const ConfigEditorDialog: React.FC<Props> = ({
export const ConfigEditorDialog = ({
config,
configSchema,
isLoading,
Expand All @@ -35,7 +35,7 @@ export const ConfigEditorDialog: React.FC<Props> = ({
title,
saveText,
isOpen,
}) => {
}: Props) => {
return (
<Dialog
isOpen={isOpen}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ export const CodeMirrorInDialogStyle = createGlobalStyle`
}
`;

export const ConfigEditorWithSchema: React.FC<Props> = ({
export const ConfigEditorWithSchema = ({
isLoading,
identifier,
config,
onConfigChange,
configSchema,
}) => {
}: Props) => {
const editorSplitPanelContainer = React.useRef<SplitPanelContainer | null>(null);
const [editorHelpContext, setEditorHelpContext] = React.useState<ConfigEditorHelpContext | null>(
null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ export interface CursorPaginationProps {
reset: () => void;
}

export const CursorPaginationControls: React.FC<CursorPaginationProps> = ({
export const CursorPaginationControls = ({
hasPrevCursor,
hasNextCursor,
popCursor,
advanceCursor,
}) => {
}: CursorPaginationProps) => {
return (
<CursorControlsContainer>
<Button disabled={!hasPrevCursor} icon={<Icon name="arrow_back" />} onClick={popCursor}>
Expand All @@ -34,12 +34,12 @@ export const CursorPaginationControls: React.FC<CursorPaginationProps> = ({
);
};

export const CursorHistoryControls: React.FC<CursorPaginationProps> = ({
export const CursorHistoryControls = ({
hasPrevCursor,
hasNextCursor,
popCursor,
advanceCursor,
}) => {
}: CursorPaginationProps) => {
return (
<CursorControlsContainer>
<Button icon={<Icon name="arrow_back" />} disabled={!hasPrevCursor} onClick={popCursor}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ interface HeaderProps {
label: React.ReactNode;
}

export const DialogHeader: React.FC<HeaderProps> = (props) => {
export const DialogHeader = (props: HeaderProps) => {
const {icon, label} = props;
return (
<Box background={Colors.White} padding={{vertical: 16, horizontal: 20}} border="bottom">
Expand Down Expand Up @@ -69,11 +69,7 @@ interface DialogFooterProps {
left?: React.ReactNode;
}

export const DialogFooter: React.FC<DialogFooterProps> = ({
children,
left,
topBorder,
}: DialogFooterProps) => {
export const DialogFooter = ({children, left, topBorder}: DialogFooterProps) => {
return (
<Box
padding={{bottom: 16, top: topBorder ? 16 : 8, horizontal: 20}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {IconName, Icon, IconWrapper} from './Icon';

interface Props extends React.ComponentProps<typeof BlueprintMenu> {}

export const Menu: React.FC<Props> = (props) => {
export const Menu = (props: Props) => {
return <StyledMenu {...props} />;
};

Expand Down Expand Up @@ -67,7 +67,7 @@ interface ItemProps
extends CommonMenuItemProps,
Omit<React.ComponentProps<typeof BlueprintMenuItem>, 'href' | 'icon'> {}

export const MenuItem: React.FC<ItemProps> = (props) => {
export const MenuItem = (props: ItemProps) => {
const {icon, intent, ...rest} = props;
return (
<StyledMenuItem
Expand All @@ -88,7 +88,7 @@ interface MenuExternalLinkProps
/**
* If you want to use a menu item as a link, use `MenuLink` and provide a `to` prop.
*/
export const MenuExternalLink: React.FC<MenuExternalLinkProps> = (props) => {
export const MenuExternalLink = (props: MenuExternalLinkProps) => {
const {icon, intent = 'none', ...rest} = props;
return (
<StyledMenuItem
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ import styled from 'styled-components';

import {calculateMiddleTruncation} from './calculateMiddleTruncation';

interface Props {
text: string;
showTitle?: boolean;
}

/**
* A component that performs middle truncation on a given string, based on the evaluated width
* of a container div.
Expand All @@ -14,53 +19,51 @@ import {calculateMiddleTruncation} from './calculateMiddleTruncation';
*
* When the DOM element resizes, the measurement and calculation steps will occur again.
*/
export const MiddleTruncate: React.FC<{text: string; showTitle?: boolean}> = React.memo(
({text, showTitle = true}) => {
// Track the font style and target maximum width. `null` means no measurement has
// taken place yet.
const [truncatedText, setTruncatedText] = React.useState<string | null>(null);

// An element that renders the full text into the container, for the purpose of
// measuring the maximum available/necessary width for our truncated string.
const measure = React.useRef<HTMLDivElement>(null);

// Given the target font style and allotted width, calculate the largest possible middle-
// truncated string.
const calculateTargetStyle = React.useCallback(() => {
if (measure.current) {
setTruncatedText(calculateMiddleTruncatedText(measure.current, text));
}
}, [text]);

// Use a layout effect to trigger the process of calculating the truncated text, for the
// initial render.
React.useLayoutEffect(() => {
calculateTargetStyle();
}, [calculateTargetStyle]);

// If the container has just been resized, recalculate.
useResizeObserver(measure.current, () => {
calculateTargetStyle();
});

// Copy the full text, not just the truncated version shown in the DOM.
const handleCopy = React.useCallback(
(e: React.ClipboardEvent<HTMLDivElement>) => {
e.preventDefault();
const clipboardAPI = navigator.clipboard;
clipboardAPI.writeText(text);
},
[text],
);

return (
<Container onCopy={handleCopy} title={showTitle ? text : undefined}>
<span>{truncatedText}</span>
<MeasureWidth ref={measure}>{text}</MeasureWidth>
</Container>
);
},
);
export const MiddleTruncate = ({text, showTitle = true}: Props) => {
// Track the font style and target maximum width. `null` means no measurement has
// taken place yet.
const [truncatedText, setTruncatedText] = React.useState<string | null>(null);

// An element that renders the full text into the container, for the purpose of
// measuring the maximum available/necessary width for our truncated string.
const measure = React.useRef<HTMLDivElement>(null);

// Given the target font style and allotted width, calculate the largest possible middle-
// truncated string.
const calculateTargetStyle = React.useCallback(() => {
if (measure.current) {
setTruncatedText(calculateMiddleTruncatedText(measure.current, text));
}
}, [text]);

// Use a layout effect to trigger the process of calculating the truncated text, for the
// initial render.
React.useLayoutEffect(() => {
calculateTargetStyle();
}, [calculateTargetStyle]);

// If the container has just been resized, recalculate.
useResizeObserver(measure.current, () => {
calculateTargetStyle();
});

// Copy the full text, not just the truncated version shown in the DOM.
const handleCopy = React.useCallback(
(e: React.ClipboardEvent<HTMLDivElement>) => {
e.preventDefault();
const clipboardAPI = navigator.clipboard;
clipboardAPI.writeText(text);
},
[text],
);

return (
<Container onCopy={handleCopy} title={showTitle ? text : undefined}>
<span>{truncatedText}</span>
<MeasureWidth ref={measure}>{text}</MeasureWidth>
</Container>
);
};

// An invisible target element that contains the full, no-wrapped text. This is used
// to measure the maximum available width for our truncated string.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ export type NonIdealStateProps = React.DetailedHTMLProps<
shrinkable?: boolean;
};

export const NonIdealState: React.FC<NonIdealStateProps> = ({
export const NonIdealState = ({
title,
description,
icon,
action,
shrinkable,
}) => {
}: NonIdealStateProps) => {
const singleContentElement = [title, description, action].filter(Boolean).length === 1;

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type Props = {
width?: CSSProperties['width'];
} & ObjectType;

export const ProductTour: React.FC<Props> = ({
export const ProductTour = ({
title,
description,
actions,
Expand All @@ -46,7 +46,7 @@ export const ProductTour: React.FC<Props> = ({
video,
object,
width = '260px',
}) => {
}: Props) => {
const media = React.useMemo(() => {
if (img) {
return <img src={img} style={{borderRadius: '6px'}} />;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ import styled from 'styled-components';

import {Colors} from './Colors';

export const ProgressBar: React.FC<ProgressBarProps & {fillColor?: string}> = ({
fillColor = Colors.Gray600,
...rest
}) => {
interface Props extends ProgressBarProps {
fillColor?: string;
}

export const ProgressBar = ({fillColor = Colors.Gray600, ...rest}: Props) => {
return (
<StyledProgressBar
{...rest}
Expand Down
Loading

2 comments on commit 684a01c

@github-actions
Copy link

Choose a reason for hiding this comment

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

Deploy preview for dagit-storybook ready!

✅ Preview
https://dagit-storybook-mykd695r3-elementl.vercel.app

Built with commit 684a01c.
This pull request is being automatically deployed with vercel-action

@github-actions
Copy link

Choose a reason for hiding this comment

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

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-2n6bdwpi8-elementl.vercel.app

Built with commit 684a01c.
This pull request is being automatically deployed with vercel-action

Please sign in to comment.