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

[eslint-config] break: set 'react-hooks/exhaustive-deps' to error #6624

Merged
merged 2 commits into from
Jan 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions packages/core/src/components/context-menu/contextMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -146,16 +146,17 @@ export const ContextMenu: React.FC<ContextMenuProps> = React.forwardRef<any, Con
React.useEffect(() => {
setIsOpen(false);
tooltipCtxDispatch({ type: "RESET_DISABLED_STATE" });
}, [disabled]);
}, [disabled, tooltipCtxDispatch]);

const handlePopoverClose = React.useCallback(() => {
setIsOpen(false);
setMouseEvent(undefined);
tooltipCtxDispatch({ type: "RESET_DISABLED_STATE" });
onClose?.();
}, []);
}, [onClose, tooltipCtxDispatch]);

// if the menu was just opened, we should check for dark theme (but don't do this on every render)
// eslint-disable-next-line react-hooks/exhaustive-deps
const isDarkTheme = React.useMemo(() => Utils.isDarkTheme(childRef.current), [childRef, isOpen]);

const contentProps: ContextMenuContentProps = React.useMemo(
Expand Down Expand Up @@ -223,7 +224,7 @@ export const ContextMenu: React.FC<ContextMenuProps> = React.forwardRef<any, Con

onContextMenu?.(e);
},
[children, onContextMenu, menuContent, renderMenu],
[disabled, children, content, onContextMenu, tooltipCtxDispatch, renderMenu],
);

const containerClassName = classNames(className, Classes.CONTEXT_MENU);
Expand Down
13 changes: 8 additions & 5 deletions packages/core/src/components/context-menu/contextMenuPopover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,14 @@ export const ContextMenuPopover = React.memo(function _ContextMenuPopover(props:
[targetOffset],
);

const handleInteraction = React.useCallback((nextOpenState: boolean) => {
if (!nextOpenState) {
onClose?.();
}
}, []);
const handleInteraction = React.useCallback(
(nextOpenState: boolean) => {
if (!nextOpenState) {
onClose?.();
}
},
[onClose],
);

return (
<Popover
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,12 @@ export function hideContextMenu() {
* A simple wrapper around `ContextMenuPopover` which is open by default and uncontrolled.
* It closes when a user clicks outside the popover.
*/
function UncontrolledContextMenuPopover(props: Omit<ContextMenuPopoverProps, "isOpen">) {
function UncontrolledContextMenuPopover({ onClose, ...props }: Omit<ContextMenuPopoverProps, "isOpen">) {
const [isOpen, setIsOpen] = React.useState(true);
const handleClose = React.useCallback(() => {
setIsOpen(false);
props.onClose?.();
}, [props.onClose]);
onClose?.();
}, [onClose]);

return <ContextMenuPopover isOpen={isOpen} {...props} onClose={handleClose} />;
}
6 changes: 1 addition & 5 deletions packages/core/src/components/panel-stack2/panelStack2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,16 +123,12 @@ export const PanelStack2: PanelStack2Component = <T extends Panel<object>>(props
);
const handlePanelClose = React.useCallback(
(panel: T) => {
// only remove this panel if it is at the top and not the only one.
if (stack[0] !== panel || stack.length <= 1) {
return;
}
onClose?.(panel);
if (!isControlled) {
setLocalStack(prevStack => prevStack.slice(1));
}
},
[stack, onClose, isControlled],
[onClose, isControlled],
);

// early return, after all hooks are called
Expand Down
37 changes: 24 additions & 13 deletions packages/core/src/components/panel-stack2/panelView2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,24 +52,35 @@ interface PanelView2Component {
}

// eslint-disable-next-line @typescript-eslint/ban-types
export const PanelView2: PanelView2Component = <T extends Panel<object>>(props: PanelView2Props<T>) => {
const handleClose = React.useCallback(() => props.onClose(props.panel), [props.onClose, props.panel]);
export const PanelView2: PanelView2Component = <T extends Panel<object>>({
panel,
onClose,
onOpen,
previousPanel,
showHeader,
}: PanelView2Props<T>) => {
const handleClose = React.useCallback(() => {
// only remove this panel if it is not the only one.
if (previousPanel !== undefined) {
onClose(panel);
}
}, [onClose, panel, previousPanel]);

const maybeBackButton =
props.previousPanel === undefined ? null : (
previousPanel === undefined ? null : (
<Button
aria-label="Back"
className={Classes.PANEL_STACK_HEADER_BACK}
icon="chevron-left"
minimal={true}
onClick={handleClose}
small={true}
text={props.previousPanel.title}
title={props.previousPanel.htmlTitle}
text={previousPanel.title}
title={previousPanel.htmlTitle}
/>
);

// `props.panel.renderPanel` is simply a function that returns a JSX.Element. It may be an FC which
// `panel.renderPanel` is simply a function that returns a JSX.Element. It may be an FC which
// uses hooks. In order to avoid React errors due to inconsistent hook calls, we must encapsulate
// those hooks with their own lifecycle through a very simple wrapper component.
const PanelWrapper: React.FC = React.useMemo(
Expand All @@ -78,22 +89,22 @@ export const PanelView2: PanelView2Component = <T extends Panel<object>>(props:
// instantiated with a type unrelated to our generic constraint `T` here. We know
// we're sending the right values here though, and it makes the consumer API for this
// component type safe, so it's ok to do this...
props.panel.renderPanel({
panel.renderPanel({
closePanel: handleClose,
openPanel: props.onOpen,
...props.panel.props,
openPanel: onOpen,
...panel.props,
} as PanelProps<T>),
[props.panel, props.onOpen],
[panel, handleClose, onOpen],
);

return (
<div className={Classes.PANEL_STACK2_VIEW}>
{props.showHeader && (
{showHeader && (
<div className={Classes.PANEL_STACK2_HEADER}>
{/* two <span> tags here ensure title is centered as long as possible, with `flex: 1` styling */}
<span>{maybeBackButton}</span>
<Text className={Classes.HEADING} ellipsize={true} title={props.panel.htmlTitle}>
{props.panel.title}
<Text className={Classes.HEADING} ellipsize={true} title={panel.htmlTitle}>
{panel.title}
</Text>
<span />
</div>
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/hooks/hotkeys/useHotkeys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export function useHotkeys(keys: readonly HotkeyConfig[], options: UseHotkeysOpt
document!.removeEventListener("keydown", handleGlobalKeyDown);
document!.removeEventListener("keyup", handleGlobalKeyUp);
};
}, [handleGlobalKeyDown, handleGlobalKeyUp]);
}, [document, handleGlobalKeyDown, handleGlobalKeyUp]);

return { handleKeyDown: handleLocalKeyDown, handleKeyUp: handleLocalKeyUp };
}
Expand Down
2 changes: 1 addition & 1 deletion packages/datetime/src/components/date-input/dateInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
* package instead.
*/

/* eslint-disable deprecation/deprecation, @blueprintjs/no-deprecated-components */
/* eslint-disable deprecation/deprecation, @blueprintjs/no-deprecated-components, react-hooks/exhaustive-deps */

import classNames from "classnames";
import * as React from "react";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,7 @@ export const DateInput3: React.FC<DateInput3Props> = React.memo(function _DateIn
isInputFocused,
maybeTimezonePicker,
placeholder,
popoverId,
popoverProps.targetTagName,
rightElement,
shouldShowErrorStyling,
Expand Down
15 changes: 8 additions & 7 deletions packages/docs-app/src/components/clickToCopy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export interface ClickToCopyProps extends Props, React.RefAttributes<any>, HTMLD
* The message is reset to default when the user mouses off the element after copying it.
*/
export const ClickToCopy: React.FC<ClickToCopyProps> = React.forwardRef<any, ClickToCopyProps>((props, ref) => {
const { className, children, copiedClassName, value } = props;
const { className, children, copiedClassName, onClick, onMouseLeave, onKeyDown, value } = props;
const [hasCopied, setHasCopied] = React.useState(false);
const inputRef = React.useRef<HTMLInputElement>();

Expand All @@ -58,33 +58,34 @@ export const ClickToCopy: React.FC<ClickToCopyProps> = React.forwardRef<any, Cli
const handleClick = React.useCallback(
async (e: React.MouseEvent<HTMLDivElement>) => {
await copy();
props.onClick?.(e);
onClick?.(e);
},
[copy, props.onClick],
[copy, onClick],
);

const handleMouseLeave = React.useCallback(
(e: React.MouseEvent<HTMLDivElement>) => {
setHasCopied(false);
props.onMouseLeave?.(e);
onMouseLeave?.(e);
},
[props.onMouseLeave],
[onMouseLeave],
);

const handleInputBlur = React.useCallback(() => {
setHasCopied(false);
}, []);

// eslint-disable-next-line react-hooks/exhaustive-deps
const handleKeyDown = React.useCallback(
createKeyEventHandler(
{
Enter: copy,
Space: copy,
all: props.onKeyDown,
all: onKeyDown,
},
true,
),
[copy, props.onKeyDown],
[copy, onKeyDown],
);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ export interface IntentSelectProps {
showClearButton?: boolean;
}

export const IntentSelect: React.FC<IntentSelectProps> = props => {
const handleChange = handleValueChange(props.onChange);
const handleClear = React.useCallback(() => props.onChange("none"), []);
export const IntentSelect: React.FC<IntentSelectProps> = ({ label, intent, showClearButton, onChange }) => {
const handleChange = handleValueChange(onChange);
const handleClear = React.useCallback(() => onChange("none"), [onChange]);
return (
<FormGroup label={props.label}>
<FormGroup label={label}>
<ControlGroup>
<HTMLSelect value={props.intent} onChange={handleChange} options={INTENTS} fill={true} />
{props.showClearButton && (
<Button aria-label="Clear" disabled={props.intent === "none"} icon="cross" onClick={handleClear} />
<HTMLSelect value={intent} onChange={handleChange} options={INTENTS} fill={true} />
{showClearButton && (
<Button aria-label="Clear" disabled={intent === "none"} icon="cross" onClick={handleClear} />
)}
</ControlGroup>
</FormGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,30 +28,36 @@ export const ContextMenuPopoverExample: React.FC<ExampleProps> = props => {
hideContextMenu();
}, []);

const menu = (
<Menu>
<MenuItem icon="cross-circle" intent="danger" text="Click me to close" onClick={handleClose} />
<MenuItem icon="search-around" text="Search around..." />
<MenuItem icon="search" text="Object viewer" />
<MenuItem icon="graph-remove" text="Remove" />
<MenuItem icon="group-objects" text="Group" />
<MenuDivider />
<MenuItem disabled={true} text="Clicked on node" />
</Menu>
const menu = React.useMemo(
() => (
<Menu>
<MenuItem icon="cross-circle" intent="danger" text="Click me to close" onClick={handleClose} />
<MenuItem icon="search-around" text="Search around..." />
<MenuItem icon="search" text="Object viewer" />
<MenuItem icon="graph-remove" text="Remove" />
<MenuItem icon="group-objects" text="Group" />
<MenuDivider />
<MenuItem disabled={true} text="Clicked on node" />
</Menu>
),
[handleClose],
);

const handleContextMenu = React.useCallback((event: React.MouseEvent<HTMLElement>) => {
event.preventDefault();
showContextMenu({
content: menu,
onClose: handleClose,
targetOffset: {
left: event.clientX,
top: event.clientY,
},
});
setIsOpen(true);
}, []);
const handleContextMenu = React.useCallback(
(event: React.MouseEvent<HTMLElement>) => {
event.preventDefault();
showContextMenu({
content: menu,
onClose: handleClose,
targetOffset: {
left: event.clientX,
top: event.clientY,
},
});
setIsOpen(true);
},
[handleClose, menu],
);

return (
<Example className="docs-context-menu-example" options={false} {...props}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ function ButtonWithDialog({
...props
}: Omit<DialogProps, "isOpen"> & { buttonText: string; footerStyle: "default" | "minimal" | "none" }) {
const [isOpen, setIsOpen] = React.useState(false);
const handleButtonClick = React.useCallback(() => setIsOpen(!isOpen), []);
const handleButtonClick = React.useCallback(() => setIsOpen(!isOpen), [isOpen]);
const handleClose = React.useCallback(() => setIsOpen(false), []);
const footerActions = (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,9 @@ export const PanelStack2Example: React.FC<ExampleProps> = props => {
Array<Panel<Panel1Info | Panel2Info | Panel3Info>>
>([initialPanel]);

// eslint-disable-next-line react-hooks/exhaustive-deps
const toggleActiveOnly = React.useCallback(handleBooleanChange(setActivePanelOnly), []);
// eslint-disable-next-line react-hooks/exhaustive-deps
const toggleShowHeader = React.useCallback(handleBooleanChange(setShowHeader), []);
const addToPanelStack = React.useCallback(
(newPanel: Panel<Panel1Info | Panel2Info | Panel3Info>) => setCurrentPanelStack(stack => [...stack, newPanel]),
Expand Down
11 changes: 7 additions & 4 deletions packages/docs-theme/src/components/typescript/apiLink.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,13 @@ export interface ApiLinkProps extends Props {
*/
export const ApiLink: React.FC<ApiLinkProps> = ({ className, name }) => {
const { showApiDocs } = React.useContext(DocumentationContext);
const handleClick = React.useCallback((evt: React.MouseEvent<HTMLAnchorElement>) => {
evt.preventDefault();
showApiDocs(name);
}, []);
const handleClick = React.useCallback(
(evt: React.MouseEvent<HTMLAnchorElement>) => {
evt.preventDefault();
showApiDocs(name);
},
[name, showApiDocs],
);

return (
<a className={className} href={`#api/${name}`} onClick={handleClick}>
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-config/eslint-plugin-rules.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,5 @@
"react/no-string-refs": "error",
"react/self-closing-comp": "error",
"react-hooks/rules-of-hooks": "error",
"react-hooks/exhaustive-deps": "warn"
"react-hooks/exhaustive-deps": "error"
}
Loading