fix(eslint): resolve react-hooks/globals react-hooks/immutability issues (#7612)

Author: gribnoysup

configs/eslint-config-compass/index.js

@@ -7,11 +7,9 @@ const chaiFriendly = require('eslint-plugin-chai-friendly');
 
 // TODO(COMPASS-9459): disabling a bunch of new rules to unblock automatic updates
 const tempNewEslintRulesDisabled = {
-  'react-hooks/immutability': 'off',
   'react-hooks/refs': 'off',
   'react-hooks/set-state-in-effect': 'off',
   'react-hooks/preserve-manual-memoization': 'off',
-  'react-hooks/globals': 'off',
   'react-hooks/static-components': 'off',
   '@typescript-eslint/no-redundant-type-constituents': 'off',
   '@typescript-eslint/no-unsafe-enum-comparison': 'off',

configs/testing-library-compass/src/index.tsx

@@ -340,8 +340,13 @@ function createWrapper(
   const StoreGetter: React.FunctionComponent = ({ children }) => {
     const store = useStore();
     const actions = useConnectionActions();
-    wrapperState.connectionsStore.getState = store.getState.bind(store);
-    wrapperState.connectionsStore.actions = actions;
+    // We're breaking the rules of hooks on purpose here to expose the values
+    // outside of the render for testing purposes
+    // eslint-disable-next-line react-hooks/immutability
+    Object.assign(wrapperState.connectionsStore, {
+      getState: store.getState.bind(store),
+      actions,
+    });
     return <>{children}</>;
   };
   const logger = {
@@ -618,6 +623,9 @@ function createPluginWrapper<
 ) {
   const ref: { current: PluginContext } = { current: {} as any };
   function ComponentWithProvider({ children, ...props }: any) {
+    // We're breaking the rules of hooks on purpose here to expose the ref
+    // outside of the render
+    // eslint-disable-next-line react-hooks/immutability
     const plugin = (ref.current = Plugin.useActivate(
       initialPluginProps ?? ({} as any)
     ));

packages/compass-assistant/src/compass-assistant-drawer.tsx

@@ -120,6 +120,10 @@ export const ClearChatButton: React.FunctionComponent<{
     if (confirmed) {
       await stop();
       clearError();
+      // Instead of breaking React rules, we should probably expose the "clear"
+      // as an interface on the chat class. Otherwise it's kinda expected taht
+      // we "mutate" messages directly to update the state
+      // eslint-disable-next-line react-hooks/immutability
       chat.messages = chat.messages.filter(
         (message) => message.metadata?.isPermanent
       );

packages/compass-components/src/components/drawer-portal.spec.tsx

@@ -21,6 +21,8 @@ describe('DrawerSection', function () {
 
     function TestDrawer() {
       const [count, _setCount] = useState(0);
+      // Exposed for testing purposes
+      // eslint-disable-next-line react-hooks/globals
       setCount = _setCount;
       return (
         <DrawerContentProvider>

packages/compass-components/src/components/drawer-portal.tsx

@@ -51,6 +51,9 @@ type DrawerActionsContextValue = {
     closeDrawer: () => void;
     updateToolbarData: (data: DrawerSectionProps) => void;
     removeToolbarData: (id: string) => void;
+    setCurrent: (
+      fn: (current: DrawerActionsContextValue['current']) => void
+    ) => void;
   };
 };
 
@@ -78,6 +81,7 @@ const DrawerActionsContext = React.createContext<DrawerActionsContextValue>({
     closeDrawer: () => undefined,
     updateToolbarData: () => undefined,
     removeToolbarData: () => undefined,
+    setCurrent: () => undefined,
   },
 });
 
@@ -124,7 +128,7 @@ export const DrawerContentProvider: React.FunctionComponent<{
     useState<DrawerOpenStateContextValue>(false);
   const [drawerCurrentTab, setDrawerCurrentTab] =
     useState<DrawerCurrentTabStateContextValue>(null);
-  const drawerActions = useRef({
+  const drawerActions = useRef<DrawerActionsContextValue['current']>({
     openDrawer: () => undefined,
     closeDrawer: () => undefined,
     updateToolbarData: (data: DrawerSectionProps) => {
@@ -147,6 +151,9 @@ export const DrawerContentProvider: React.FunctionComponent<{
         });
       });
     },
+    setCurrent: (fn) => {
+      fn(drawerActions.current);
+    },
   });
 
   const prevDrawerCurrentTabRef = React.useRef<string | null>(null);
@@ -187,11 +194,14 @@ export const DrawerContentProvider: React.FunctionComponent<{
 
 const DrawerContextGrabber: React.FunctionComponent = ({ children }) => {
   const drawerToolbarContext = useDrawerToolbarContext();
-  const actions = useContext(DrawerActionsContext);
   const openStateSetter = useContext(DrawerSetOpenStateContext);
   const currentTabSetter = useContext(DrawerSetCurrentTabContext);
-  actions.current.openDrawer = drawerToolbarContext.openDrawer;
-  actions.current.closeDrawer = drawerToolbarContext.closeDrawer;
+  const actions = useContext(DrawerActionsContext);
+
+  actions.current.setCurrent((current) => {
+    current.openDrawer = drawerToolbarContext.openDrawer;
+    current.closeDrawer = drawerToolbarContext.closeDrawer;
+  });
 
   useEffect(() => {
     openStateSetter(drawerToolbarContext.isDrawerOpen);

packages/compass-connections/src/hooks/connection-tab-theme-provider.spec.tsx

@@ -31,6 +31,8 @@ describe('ConnectionThemeProvider', function () {
       let capturedTheme: ReturnType<typeof useTabTheme> = undefined;
 
       const TestComponent = () => {
+        // Doing this to test the value
+        // eslint-disable-next-line react-hooks/globals
         capturedTheme = useTabTheme();
         return null;
       };
@@ -53,6 +55,8 @@ describe('ConnectionThemeProvider', function () {
       let capturedTheme: ReturnType<typeof useTabTheme> = undefined;
 
       const TestComponent = () => {
+        // Doing this to test the value
+        // eslint-disable-next-line react-hooks/globals
         capturedTheme = useTabTheme();
         return null;
       };
@@ -87,6 +91,8 @@ describe('ConnectionThemeProvider', function () {
       let capturedTheme: ReturnType<typeof useTabTheme> = undefined;
 
       const TestComponent = () => {
+        // Doing this to test the value
+        // eslint-disable-next-line react-hooks/globals
         capturedTheme = useTabTheme();
         return null;
       };
@@ -119,6 +125,8 @@ describe('ConnectionThemeProvider', function () {
       };
 
       const TestComponent = () => {
+        // Doing this to test the value
+        // eslint-disable-next-line react-hooks/globals
         capturedTheme = useTabTheme();
         return null;
       };
@@ -151,6 +159,8 @@ describe('ConnectionThemeProvider', function () {
       };
 
       const TestComponent = () => {
+        // Doing this to test the value
+        // eslint-disable-next-line react-hooks/globals
         capturedTheme = useTabTheme();
         return <div>Theme consumer</div>;
       };

packages/compass-web/sandbox/sandbox-atlas-sign-in.tsx

@@ -78,7 +78,7 @@ export function useAtlasProxySignIn(): AtlasLoginReturnValue {
     null
   );
 
-  const signIn = ((window as any).__signIn = useCallback(async () => {
+  const signIn = useCallback(async () => {
     try {
       const { projectId } = await fetch('/authenticate', {
         method: 'POST',
@@ -94,9 +94,9 @@ export function useAtlasProxySignIn(): AtlasLoginReturnValue {
         description: (err as any).message,
       });
     }
-  }, []));
+  }, []);
 
-  const signOut = ((window as any).__signOut = useCallback(() => {
+  const signOut = useCallback(() => {
     return fetch('/logout').then(
       () => {
         window.location.reload();
@@ -105,7 +105,11 @@ export function useAtlasProxySignIn(): AtlasLoginReturnValue {
         // noop
       }
     );
-  }, []));
+  }, []);
+
+  // Global is modified only for local dev convenience
+  // eslint-disable-next-line react-hooks/immutability
+  Object.assign(window, { __signIn: signIn, __signOut: signOut });
 
   useEffect(() => {
     let mounted = true;

packages/compass-workspaces/src/index.spec.tsx

@@ -65,6 +65,8 @@ describe('WorkspacesPlugin', function () {
 
   async function renderPlugin() {
     function OpenWorkspaceFnsGetter() {
+      // exposed outside of render loop for testing
+      // eslint-disable-next-line react-hooks/globals
       openFns = useOpenWorkspace();
       return null;
     }