Remove conditional usage of React hooks (#7907)

* Remove conditional usage of React hooks * Restore useMemo() for the WizardNav * Restore empty checks for points
patternfly · Aug 31, 2022 · a49a0ff · a49a0ff
1 parent e688e3b
commit a49a0ff
Show file tree

Hide file tree

Showing 14 changed files with 127 additions and 109 deletions.
diff --git a/.eslintrc.json b/.eslintrc.json
@@ -6,15 +6,14 @@
     },
     "plugins": [
         "@typescript-eslint",
-        "react",
-        "react-hooks",
         "prettier",
         "patternfly-react"
     ],
     "extends": [
         "eslint:recommended",
         "plugin:@typescript-eslint/recommended",
-        "plugin:react/recommended"
+        "plugin:react/recommended",
+        "plugin:react-hooks/recommended"
     ],
     "parserOptions": {
         "sourceType": "module",

diff --git a/packages/react-charts/src/components/ChartUtils/chart-patterns.tsx b/packages/react-charts/src/components/ChartUtils/chart-patterns.tsx
@@ -331,6 +331,9 @@ export const getDefaultPatternProps = ({
   let patternId;
 
   if (isPatternDefs) {
+    // TODO: React hooks cannot be used in regular functions (https://reactjs.org/docs/hooks-rules.html#only-call-hooks-from-react-functions).
+    // This call to 'useMemo()` needs to be replaced with something else, or this function needs to be converted to a React component.
+    // eslint-disable-next-line react-hooks/rules-of-hooks
     patternId = React.useMemo(() => getPatternId(), []);
     defaultPatternScale = getDefaultPatternScale({
       colorScale: defaultColorScale,

diff --git a/packages/react-core/src/components/DualListSelector/DualListSelectorControl.tsx b/packages/react-core/src/components/DualListSelector/DualListSelectorControl.tsx
@@ -37,7 +37,8 @@ export const DualListSelectorControlBase: React.FunctionComponent<DualListSelect
   tooltipProps = {} as any,
   ...props
 }: DualListSelectorControlProps) => {
-  const ref = innerRef || React.useRef(null);
+  const privateRef = React.useRef(null);
+  const ref = innerRef || privateRef;
   return (
     <div className={css('pf-c-dual-list-selector__controls-item', className)} {...props}>
       <Button

diff --git a/packages/react-core/src/components/DualListSelector/DualListSelectorControlsWrapper.tsx b/packages/react-core/src/components/DualListSelector/DualListSelectorControlsWrapper.tsx
@@ -23,7 +23,8 @@ export const DualListSelectorControlsWrapperBase: React.FunctionComponent<DualLi
   'aria-label': ariaLabel = 'Controls for moving options between lists',
   ...props
 }: DualListSelectorControlsWrapperProps) => {
-  const wrapperRef = innerRef || React.useRef(null);
+  const ref = React.useRef(null);
+  const wrapperRef = innerRef || ref;
   // Adds keyboard navigation to the dynamically built dual list selector controls. Works when controls are dynamically built
   // as well as when they are passed in via children.
   const handleKeys = (event: KeyboardEvent) => {

diff --git a/packages/react-core/src/components/DualListSelector/DualListSelectorListItem.tsx b/packages/react-core/src/components/DualListSelector/DualListSelectorListItem.tsx
@@ -47,7 +47,8 @@ export const DualListSelectorListItemBase: React.FunctionComponent<DualListSelec
   draggableButtonAriaLabel = 'Reorder option',
   ...props
 }: DualListSelectorListItemProps) => {
-  const ref = innerRef || React.useRef<HTMLLIElement>(null);
+  const privateRef = React.useRef<HTMLLIElement>(null);
+  const ref = innerRef || privateRef;
   const { setFocusedOption } = React.useContext(DualListSelectorListContext);
 
   return (

diff --git a/packages/react-core/src/components/DualListSelector/DualListSelectorListWrapper.tsx b/packages/react-core/src/components/DualListSelector/DualListSelectorListWrapper.tsx
@@ -42,7 +42,8 @@ export const DualListSelectorListWrapperBase: React.FunctionComponent<DualListSe
   ...props
 }: DualListSelectorListWrapperProps) => {
   const [focusedOption, setFocusedOption] = React.useState('');
-  const menuRef = innerRef || React.useRef(null);
+  const ref = React.useRef(null);
+  const menuRef = innerRef || ref;
   const { isTree } = React.useContext(DualListSelectorContext);
 
   // sets up keyboard focus handling for the dual list selector menu child of the pane. This keyboard

diff --git a/packages/react-core/src/components/InputGroup/InputGroup.tsx b/packages/react-core/src/components/InputGroup/InputGroup.tsx
@@ -25,7 +25,8 @@ export const InputGroup: React.FunctionComponent<InputGroupProps> = ({
     (child: any) => !formCtrls.includes(child.type.displayName) && child.props.id
   ) as React.ReactElement<{ id: string }>;
 
-  const inputGroupRef = innerRef || React.useRef(null);
+  const ref = React.useRef(null);
+  const inputGroupRef = innerRef || ref;
 
   return (
     <div ref={inputGroupRef} className={css(styles.inputGroup, className)} {...props}>

diff --git a/packages/react-core/src/components/SearchInput/SearchInput.tsx b/packages/react-core/src/components/SearchInput/SearchInput.tsx
@@ -125,7 +125,8 @@ const SearchInputBase: React.FunctionComponent<SearchInputProps> = ({
   const [isSearchMenuOpen, setIsSearchMenuOpen] = React.useState(false);
   const [searchValue, setSearchValue] = React.useState(value);
   const searchInputRef = React.useRef(null);
-  const searchInputInputRef = innerRef || React.useRef(null);
+  const ref = React.useRef(null);
+  const searchInputInputRef = innerRef || ref;
 
   React.useEffect(() => {
     setSearchValue(value);

diff --git a/packages/react-core/src/components/TextInputGroup/TextInputGroup.tsx b/packages/react-core/src/components/TextInputGroup/TextInputGroup.tsx
@@ -27,7 +27,8 @@ export const TextInputGroup: React.FunctionComponent<TextInputGroupProps> = ({
   innerRef,
   ...props
 }: TextInputGroupProps) => {
-  const textInputGroupRef = innerRef || React.useRef(null);
+  const ref = React.useRef(null);
+  const textInputGroupRef = innerRef || ref;
 
   return (
     <TextInputGroupContext.Provider value={{ isDisabled }}>

diff --git a/packages/react-core/src/components/TextInputGroup/TextInputGroupMain.tsx b/packages/react-core/src/components/TextInputGroup/TextInputGroupMain.tsx
@@ -57,7 +57,8 @@ export const TextInputGroupMain: React.FunctionComponent<TextInputGroupMainProps
   ...props
 }: TextInputGroupMainProps) => {
   const { isDisabled } = React.useContext(TextInputGroupContext);
-  const textInputGroupInputInputRef = innerRef || React.useRef(null);
+  const ref = React.useRef(null);
+  const textInputGroupInputInputRef = innerRef || ref;
 
   const handleChange = (event: React.FormEvent<HTMLInputElement>) => {
     onChange(event.currentTarget.value, event);

diff --git a/packages/react-core/src/next/components/Wizard/WizardToggle.tsx b/packages/react-core/src/next/components/Wizard/WizardToggle.tsx
@@ -84,95 +84,97 @@ export const WizardToggle = ({
         );
       });
 
-  const wizardNav = isCustomWizardNav(nav)
-    ? nav(isNavOpen, steps, activeStep, goToStepByIndex)
-    : React.useMemo(() => {
-        const props = {
-          isOpen: isNavOpen,
-          'aria-label': nav?.ariaLabel || 'Wizard nav',
-          ...(nav?.ariaLabelledBy && { 'aria-labelledby': nav?.ariaLabelledBy })
-        };
+  const wizardNav = React.useMemo(() => {
+    if (isCustomWizardNav(nav)) {
+      return nav(isNavOpen, steps, activeStep, goToStepByIndex);
+    }
+
+    const props = {
+      isOpen: isNavOpen,
+      'aria-label': nav?.ariaLabel || 'Wizard nav',
+      ...(nav?.ariaLabelledBy && { 'aria-labelledby': nav?.ariaLabelledBy })
+    };
 
-        return (
-          <WizardNav {...props}>
-            {steps.map((step, index) => {
-              const stepIndex = index + 1;
-              const stepNavItem = step.navItem && <React.Fragment key={step.id}>{step.navItem}</React.Fragment>;
-
-              if (isWizardParentStep(step)) {
-                let firstSubStepIndex;
-                let hasActiveChild = false;
-
-                const subNavItems = step.subStepIds?.map((subStepId, index) => {
-                  const subStep = steps.find(step => step.id === subStepId);
-                  const subStepIndex = steps.indexOf(subStep) + 1;
-
-                  if (index === 0) {
-                    firstSubStepIndex = subStepIndex;
-                  }
-
-                  if (activeStep?.id === subStep.id) {
-                    hasActiveChild = true;
-                  }
-
-                  return subStep.navItem ? (
-                    <React.Fragment key={subStep.id}>{subStep.navItem}</React.Fragment>
-                  ) : (
-                    <WizardNavItem
-                      key={subStep.id}
-                      id={subStep.id}
-                      content={subStep.name}
-                      isCurrent={activeStep?.id === subStep.id}
-                      isDisabled={subStep.isDisabled || (nav?.forceStepVisit && !subStep.visited)}
-                      step={subStepIndex}
-                      onNavItemClick={goToStepByIndex}
-                    />
-                  );
-                });
-
-                const hasEnabledChildren = React.Children.toArray(subNavItems).some(
-                  child => React.isValidElement(child) && !child.props.isDisabled
-                );
-
-                return (
-                  stepNavItem || (
-                    <WizardNavItem
-                      key={step.id}
-                      id={step.id}
-                      content={step.name}
-                      isExpandable={nav?.isExpandable}
-                      isCurrent={hasActiveChild}
-                      isDisabled={!hasEnabledChildren}
-                      step={firstSubStepIndex}
-                      onNavItemClick={goToStepByIndex}
-                    >
-                      <WizardNav {...props} returnList>
-                        {subNavItems}
-                      </WizardNav>
-                    </WizardNavItem>
-                  )
-                );
+    return (
+      <WizardNav {...props}>
+        {steps.map((step, index) => {
+          const stepIndex = index + 1;
+          const stepNavItem = step.navItem && <React.Fragment key={step.id}>{step.navItem}</React.Fragment>;
+
+          if (isWizardParentStep(step)) {
+            let firstSubStepIndex;
+            let hasActiveChild = false;
+
+            const subNavItems = step.subStepIds?.map((subStepId, index) => {
+              const subStep = steps.find(step => step.id === subStepId);
+              const subStepIndex = steps.indexOf(subStep) + 1;
+
+              if (index === 0) {
+                firstSubStepIndex = subStepIndex;
               }
 
-              if (isWizardBasicStep(step)) {
-                return (
-                  stepNavItem || (
-                    <WizardNavItem
-                      key={step.id}
-                      id={step.id}
-                      content={step.name}
-                      isCurrent={activeStep?.id === step.id}
-                      isDisabled={step.isDisabled || (nav?.forceStepVisit && !step.visited)}
-                      step={stepIndex}
-                      onNavItemClick={goToStepByIndex}
-                    />
-                  )
-                );
+              if (activeStep?.id === subStep.id) {
+                hasActiveChild = true;
               }
-            })}
-          </WizardNav>
-        );
-      }, [activeStep?.id, goToStepByIndex, isNavOpen, nav, steps]);
+
+              return subStep.navItem ? (
+                <React.Fragment key={subStep.id}>{subStep.navItem}</React.Fragment>
+              ) : (
+                <WizardNavItem
+                  key={subStep.id}
+                  id={subStep.id}
+                  content={subStep.name}
+                  isCurrent={activeStep?.id === subStep.id}
+                  isDisabled={subStep.isDisabled || (nav?.forceStepVisit && !subStep.visited)}
+                  step={subStepIndex}
+                  onNavItemClick={goToStepByIndex}
+                />
+              );
+            });
+
+            const hasEnabledChildren = React.Children.toArray(subNavItems).some(
+              child => React.isValidElement(child) && !child.props.isDisabled
+            );
+
+            return (
+              stepNavItem || (
+                <WizardNavItem
+                  key={step.id}
+                  id={step.id}
+                  content={step.name}
+                  isExpandable={nav?.isExpandable}
+                  isCurrent={hasActiveChild}
+                  isDisabled={!hasEnabledChildren}
+                  step={firstSubStepIndex}
+                  onNavItemClick={goToStepByIndex}
+                >
+                  <WizardNav {...props} returnList>
+                    {subNavItems}
+                  </WizardNav>
+                </WizardNavItem>
+              )
+            );
+          }
+
+          if (isWizardBasicStep(step)) {
+            return (
+              stepNavItem || (
+                <WizardNavItem
+                  key={step.id}
+                  id={step.id}
+                  content={step.name}
+                  isCurrent={activeStep?.id === step.id}
+                  isDisabled={step.isDisabled || (nav?.forceStepVisit && !step.visited)}
+                  step={stepIndex}
+                  onNavItemClick={goToStepByIndex}
+                />
+              )
+            );
+          }
+        })}
+      </WizardNav>
+    );
+  }, [activeStep?.id, goToStepByIndex, isNavOpen, nav, steps]);
 
   return (
     <>

diff --git a/packages/react-log-viewer/src/LogViewer/LogViewer.tsx b/packages/react-log-viewer/src/LogViewer/LogViewer.tsx
@@ -100,7 +100,8 @@ const LogViewerBase: React.FunctionComponent<LogViewerProps> = memo(
     const [listKey, setListKey] = useState(1);
     const ansiUp = new AnsiUp();
 
-    const logViewerRef = innerRef || React.useRef<any>();
+    const ref = React.useRef<any>();
+    const logViewerRef = innerRef || ref;
     const containerRef = React.useRef<any>();
     let resizeTimer = null as any;
 

diff --git a/packages/react-table/src/components/TableComposable/TableComposable.tsx b/packages/react-table/src/components/TableComposable/TableComposable.tsx
@@ -95,7 +95,8 @@ const TableComposableBase: React.FunctionComponent<TableComposableProps> = ({
   selectableRowCaptionText,
   ...props
 }: TableComposableProps) => {
-  const tableRef = innerRef || React.useRef(null);
+  const ref = React.useRef(null);
+  const tableRef = innerRef || ref;
 
   const [hasSelectableRows, setHasSelectableRows] = React.useState(false);
   const [tableCaption, setTableCaption] = React.useState<JSX.Element | undefined>();

diff --git a/packages/react-topology/src/components/edges/terminals/ConnectorArrow.tsx b/packages/react-topology/src/components/edges/terminals/ConnectorArrow.tsx
@@ -25,13 +25,6 @@ const ConnectorArrow: React.FunctionComponent<ConnectorArrowProps> = ({
   size = 14,
   dragRef
 }) => {
-  if (!startPoint || !endPoint) {
-    return null;
-  }
-
-  const connectorStartPoint = getConnectorStartPoint(startPoint, endPoint, size);
-  const angleDeg = getConnectorRotationAngle(startPoint, endPoint);
-
   const polygonPoints = React.useMemo(
     () =>
       pointsStringFromPoints([
@@ -41,10 +34,21 @@ const ConnectorArrow: React.FunctionComponent<ConnectorArrowProps> = ({
       ]),
     [size]
   );
-  const boundingPoints = React.useMemo(
-    () => pointsStringFromPoints(getConnectorBoundingBox(startPoint, endPoint, size)),
-    [endPoint, size, startPoint]
-  );
+
+  const boundingPoints = React.useMemo(() => {
+    if (startPoint || !endPoint) {
+      return null;
+    }
+    return pointsStringFromPoints(getConnectorBoundingBox(startPoint, endPoint, size));
+  }, [endPoint, size, startPoint]);
+
+  if (!startPoint || !endPoint) {
+    return null;
+  }
+
+  const connectorStartPoint = getConnectorStartPoint(startPoint, endPoint, size);
+  const angleDeg = getConnectorRotationAngle(startPoint, endPoint);
+
   const classNames = css(styles.topologyConnectorArrow, className, dragRef && 'pf-m-draggable');
 
   return (