Add react-hooks/exhaustive-deps ESLint rule (#6180)

.changeset/nine-eagles-develop.md

@@ -0,0 +1,10 @@
+---
+'@keystone-ui/tooltip': patch
+'@keystone-next/auth': patch
+'@keystone-next/cloudinary': patch
+'@keystone-next/fields': patch
+'@keystone-next/fields-document': patch
+'@keystone-next/keystone': patch
+---
+
+Fixed issues with React hooks dependency arrays

.eslintrc.js

@@ -33,6 +33,7 @@ module.exports = {
     'no-undef': 'error',
     'no-unused-expressions': 'error',
     'react-hooks/rules-of-hooks': 'error',
+    'react-hooks/exhaustive-deps': 'error',
     '@typescript-eslint/no-unused-vars': [
       'error',
       {

design-system/packages/tooltip/src/Tooltip.tsx

@@ -64,8 +64,8 @@ export const Tooltip = ({
   });
 
   const tooltipId = useId();
-  const showTooltip = useCallback(() => setOpen(true), []);
-  const hideTooltip = useCallback(() => setOpen(false), []);
+  const showTooltip = useCallback(() => setOpen(true), [setOpen]);
+  const hideTooltip = useCallback(() => setOpen(false), [setOpen]);
   const internalRef = useRef<HTMLElement>(null);
 
   // avoid overriding the consumer's `onClick` handler
@@ -77,7 +77,7 @@ export const Tooltip = ({
 
       return () => triggerEl.removeEventListener('click', hideTooltip);
     }
-  }, [isOpen]);
+  }, [isOpen, hideOnClick, hideTooltip]);
 
   return (
     <Fragment>

docs/components/MobileMenu.tsx

@@ -30,7 +30,7 @@ export function MobileMenu({ handleClose }: MobileMenuProps) {
     return () => {
       document.body.removeEventListener('keydown', handleEsc);
     };
-  }, []);
+  }, [mobileNavIsOpen, handleClose]);
 
   return (
     <Fragment>

docs/components/Theme.tsx

@@ -20,7 +20,7 @@ export function Theme() {
     if (detectedTheme !== 'light') {
       setTheme(detectedTheme);
     }
-  });
+  }, []);
 
   return (
     <Global

docs/components/docs/DocumentEditorDemo.tsx

@@ -277,15 +277,15 @@ export const DocumentEditorDemo = () => {
   const isShiftPressedRef = useKeyDownRef('Shift');
   const editor = useMemo(
     () => createDocumentEditor(documentFeatures, componentBlocks, emptyObj, isShiftPressedRef),
-    [documentFeatures]
+    [documentFeatures, isShiftPressedRef]
   );
 
   // this is why we're creating the editor ourselves and not using the DocumentEditor component
   useEffect(() => {
     // we want to force normalize when the document features change so
     // that no invalid things exist after a user changes something
     Editor.normalize(editor, { force: true });
-  }, [documentFeatures]);
+  }, [editor, documentFeatures, isShiftPressedRef]);
 
   return (
     <div

docs/components/docs/TableOfContents.tsx

@@ -30,7 +30,6 @@ export function TableOfContents({
   container: React.RefObject<HTMLElement | null>;
   headings: Heading[];
 }) {
-  let allIds = headings.map(h => h.id);
   let [visibleIds, setVisibleIds] = useState<Array<string | null>>([]);
   let [lastVisibleId, setLastVisbleId] = useState<string | null>(null);
 
@@ -40,6 +39,7 @@ export function TableOfContents({
 
   useEffect(() => {
     if (container.current) {
+      let allIds = headings.map(h => h.id);
       const observer = new IntersectionObserver(entries => {
         entries.forEach(entry => {
           const targetId: string | null = entry.target.getAttribute('id');
@@ -57,7 +57,7 @@ export function TableOfContents({
       });
       return () => observer.disconnect();
     }
-  }, [container]);
+  }, [container, headings]);
 
   // catch if we're in a long gap between headings and resolve to the last available.
   let activeId = visibleIds[0] || lastVisibleId;

docs/components/primitives/Emoji.tsx

@@ -42,6 +42,7 @@ export function Emoji({ symbol, alt, ...props }: EmojiProps) {
   const posRef = useRef<HTMLElement>(null);
   const [showOnTop, setShownTop] = useState(true);
 
+  // eslint-disable-next-line react-hooks/exhaustive-deps
   useEffect(() => {
     if (posRef.current && posRef.current.offsetTop - window.pageYOffset < 50) {
       setShownTop(false);

packages/auth/src/pages/InitPage.tsx

@@ -165,7 +165,7 @@ const InitPage = ({ fieldPaths, listKey, enableWelcome }: InitPageProps) => {
       fields[fieldPath] = adminMeta.lists[listKey].fields[fieldPath];
     });
     return fields;
-  }, [fieldPaths]);
+  }, [fieldPaths, adminMeta.lists, listKey]);
 
   const [value, setValue] = useState(() => {
     let state: Record<string, any> = {};
@@ -211,7 +211,7 @@ const InitPage = ({ fieldPaths, listKey, enableWelcome }: InitPageProps) => {
         router.push((router.query.from as string | undefined) || '/');
       }
     }
-  }, [rawKeystone.authenticatedItem, router.query.from]);
+  }, [rawKeystone.authenticatedItem, enableWelcome, router]);
 
   return mode === 'init' ? (
     <SigninContainer>

packages/auth/src/pages/SigninPage.tsx

@@ -61,7 +61,7 @@ export const SigninPage = ({
     if (rawKeystone.authenticatedItem.state === 'authenticated') {
       router.push((router.query.from as string | undefined) || '/');
     }
-  }, [rawKeystone.authenticatedItem, router.query.from]);
+  }, [rawKeystone.authenticatedItem, router]);
   return (
     <SigninContainer>
       <Stack

packages/cloudinary/src/views/Field.tsx

@@ -41,6 +41,7 @@ export function Field({
   // Generate a random input key when the value changes, to ensure the file input is unmounted and
   // remounted (this is the only way to reset its value and ensure onChange will fire again if
   // the user selects the same file again)
+  // eslint-disable-next-line react-hooks/exhaustive-deps
   const inputKey = useMemo(() => Math.random(), [value]);
 
   return (

packages/fields-document/src/DocumentEditor/Toolbar.tsx

@@ -51,7 +51,6 @@ export function Toolbar({
   documentFeatures: DocumentFeatures;
   viewState?: { expanded: boolean; toggle: () => void };
 }) {
-  const ExpandIcon = viewState?.expanded ? Minimize2Icon : Maximize2Icon;
   const relationship = useContext(DocumentFieldRelationshipsContext);
   const blockComponent = useContext(ComponentBlockContext);
   const hasBlockItems = Object.entries(relationship).length || Object.keys(blockComponent).length;
@@ -119,8 +118,9 @@ export function Toolbar({
       {!!hasBlockItems && <InsertBlockMenu />}
 
       <ToolbarSeparator />
-      {useMemo(
-        () =>
+      {useMemo(() => {
+        const ExpandIcon = viewState?.expanded ? Minimize2Icon : Maximize2Icon;
+        return (
           viewState && (
             <Tooltip content={viewState.expanded ? 'Collapse' : 'Expand'} weight="subtle">
               {attrs => (
@@ -135,9 +135,9 @@ export function Toolbar({
                 </ToolbarButton>
               )}
             </Tooltip>
-          ),
-        [viewState]
-      )}
+          )
+        );
+      }, [viewState])}
     </ToolbarContainer>
   );
 }
@@ -172,7 +172,7 @@ const MarkButton = forwardRef<any, { children: ReactNode; type: Mark }>(function
         {...restProps}
       />
     );
-  }, [editor, isDisabled, isSelected, props]);
+  }, [editor, isDisabled, isSelected, props, ref]);
 });
 
 const ToolbarContainer = ({ children }: { children: ReactNode }) => {

packages/fields-document/src/DocumentEditor/blockquote.tsx

@@ -116,7 +116,7 @@ const BlockquoteButton = ({
         <QuoteIcon />
       </ToolbarButton>
     ),
-    [attrs, isDisabled, isSelected]
+    [editor, attrs, isDisabled, isSelected]
   );
 };
 export const blockquoteButton = (

packages/fields-document/src/DocumentEditor/code-block.tsx

@@ -83,7 +83,7 @@ function CodeButton({ attrs }: { attrs: {} }) {
         <CodeIcon size="small" />
       </ToolbarButton>
     ),
-    [isDisabled, isSelected, attrs]
+    [isDisabled, isSelected, attrs, editor]
   );
 }
 

packages/fields-document/src/DocumentEditor/index.tsx

@@ -230,7 +230,7 @@ export function DocumentEditor({
   const [expanded, setExpanded] = useState(false);
   const editor = useMemo(
     () => createDocumentEditor(documentFeatures, componentBlocks, relationships, isShiftPressedRef),
-    [documentFeatures, componentBlocks, relationships]
+    [documentFeatures, componentBlocks, relationships, isShiftPressedRef]
   );
 
   return (
@@ -309,6 +309,7 @@ export function DocumentEditorProvider({
   relationships: Relationships;
   documentFeatures: DocumentFeatures;
 }) {
+  // eslint-disable-next-line react-hooks/exhaustive-deps
   const identity = useMemo(() => Math.random().toString(36), [editor]);
   return (
     <Slate
@@ -392,7 +393,7 @@ export function DocumentEditorEditable({
           }
           return decorations;
         },
-        [editor]
+        [editor, componentBlocks]
       )}
       css={styles}
       autoFocus={autoFocus}

packages/fields-document/src/DocumentEditor/insert-menu.tsx

@@ -214,7 +214,7 @@ export function InsertMenu({ children, text }: { children: ReactNode; text: Text
           return;
         }
         case 'Escape': {
-          const path = ReactEditor.findPath(editor, text);
+          const path = ReactEditor.findPath(editor, stateRef.current.text);
           Transforms.unsetNodes(editor, 'insertMenu', { at: path });
           event.preventDefault();
           return;

packages/fields-document/src/DocumentEditor/lists.tsx

@@ -176,7 +176,7 @@ export const ListButton = forwardRef<
         {...restProps}
       />
     );
-  }, [props, ref, isDisabled, isSelected]);
+  }, [props, ref, isDisabled, isSelected, editor]);
 });
 
 export function nestList(editor: Editor) {

packages/fields/src/types/file/views/Field.tsx

@@ -98,6 +98,7 @@ export function Field({
   // Generate a random input key when the value changes, to ensure the file input is unmounted and
   // remounted (this is the only way to reset its value and ensure onChange will fire again if
   // the user selects the same file again)
+  // eslint-disable-next-line react-hooks/exhaustive-deps
   const inputKey = useMemo(() => Math.random(), [value]);
 
   return (

packages/fields/src/types/image/views/Field.tsx

@@ -106,6 +106,7 @@ export function Field({
   // Generate a random input key when the value changes, to ensure the file input is unmounted and
   // remounted (this is the only way to reset its value and ensure onChange will fire again if
   // the user selects the same file again)
+  // eslint-disable-next-line react-hooks/exhaustive-deps
   const inputKey = useMemo(() => Math.random(), [value]);
 
   return (

packages/keystone/src/___internal-do-not-use-will-break-in-patch/admin-ui/pages/ItemPage/index.tsx

@@ -161,6 +161,8 @@ function ItemForm({
         });
       });
   });
+  const labelFieldValue = itemGetter.data?.[list.labelField];
+  const itemId = itemGetter.data?.id!;
   return (
     <Box marginTop="xlarge">
       <GraphQLErrorNotice
@@ -200,11 +202,11 @@ function ItemForm({
             showDelete ? (
               <DeleteButton
                 list={list}
-                itemLabel={(itemGetter.data?.[list.labelField] ?? itemGetter.data?.id!) as string}
-                itemId={itemGetter.data?.id!}
+                itemLabel={(labelFieldValue ?? itemId) as string}
+                itemId={itemId}
               />
             ) : undefined,
-          [showDelete, list, itemGetter.data?.[list.labelField], itemGetter.data?.id]
+          [showDelete, list, labelFieldValue, itemId]
         )}
       />
     </Box>

packages/keystone/src/___internal-do-not-use-will-break-in-patch/admin-ui/pages/ListPage/index.tsx

@@ -74,41 +74,46 @@ let listMetaGraphqlQuery: TypedDocumentNode<
   }
 `;
 
+const storeableQueries = ['sortBy', 'fields'];
+
 function useQueryParamsFromLocalStorage(listKey: string) {
   const router = useRouter();
   const localStorageKey = `keystone.list.${listKey}.list.page.info`;
-  const storeableQueries = ['sortBy', 'fields'];
 
   const resetToDefaults = () => {
-    localStorage.removeItem(`keystone.list.${listKey}.list.page.info`);
+    localStorage.removeItem(localStorageKey);
     router.replace({
       pathname: router.pathname,
     });
   };
 
   // GET QUERY FROM CACHE IF CONDITIONS ARE RIGHT
   // MERGE QUERY PARAMS FROM CACHE WITH QUERY PARAMS FROM ROUTER
-  useEffect(() => {
-    let hasSomeQueryParamsWhichAreAboutListPage = Object.keys(router.query).some(x => {
-      return x.startsWith('!') || storeableQueries.includes(x);
-    });
-
-    if (!hasSomeQueryParamsWhichAreAboutListPage && router.isReady) {
-      const queryParamsFromLocalStorage = localStorage.getItem(localStorageKey);
-      let parsed;
-      try {
-        parsed = JSON.parse(queryParamsFromLocalStorage!);
-      } catch (err) {}
-      if (parsed) {
-        router.replace({
-          query: {
-            ...router.query,
-            ...parsed,
-          },
-        });
+  useEffect(
+    () => {
+      let hasSomeQueryParamsWhichAreAboutListPage = Object.keys(router.query).some(x => {
+        return x.startsWith('!') || storeableQueries.includes(x);
+      });
+
+      if (!hasSomeQueryParamsWhichAreAboutListPage && router.isReady) {
+        const queryParamsFromLocalStorage = localStorage.getItem(localStorageKey);
+        let parsed;
+        try {
+          parsed = JSON.parse(queryParamsFromLocalStorage!);
+        } catch (err) {}
+        if (parsed) {
+          router.replace({
+            query: {
+              ...router.query,
+              ...parsed,
+            },
+          });
+        }
       }
-    }
-  }, [localStorageKey, router.isReady]);
+    },
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+    [localStorageKey, router.isReady]
+  );
   useEffect(() => {
     let queryParamsToSerialize: Record<string, string> = {};
     Object.keys(router.query).forEach(key => {
@@ -117,12 +122,9 @@ function useQueryParamsFromLocalStorage(listKey: string) {
       }
     });
     if (Object.keys(queryParamsToSerialize).length) {
-      localStorage.setItem(
-        `keystone.list.${listKey}.list.page.info`,
-        JSON.stringify(queryParamsToSerialize)
-      );
+      localStorage.setItem(localStorageKey, JSON.stringify(queryParamsToSerialize));
     } else {
-      localStorage.removeItem(`keystone.list.${listKey}.list.page.info`);
+      localStorage.removeItem(localStorageKey);
     }
   }, [localStorageKey, router]);
 
@@ -440,7 +442,7 @@ function DeleteManyButton({
     }
   }
 `,
-      [list, selectedItems]
+      [list]
     )
   );
   const [isOpen, setIsOpen] = useState(false);