advances fixing linter

Author: johnny-quesada-developer

eslint.config.mjs

@@ -81,41 +81,141 @@ export default [
       },
     },
     rules: {
-      // Nx boundaries
+      /**
+       * Nx boundaries
+       * @see https://nx.dev/docs/features/enforce-module-boundaries
+       * */
       '@nx/enforce-module-boundaries': [
         'error',
         {
           enforceBuildableLibDependency: true,
           allow: ['^.*/eslint(\\.base)?\\.config\\.[cm]?js$'],
-          depConstraints: [{ sourceTag: '*', onlyDependOnLibsWithTags: ['*'] }],
+          depConstraints: [
+            // example: apps can depend on ui + shared libs
+            {
+              sourceTag: 'type:app',
+              onlyDependOnLibsWithTags: ['scope:ui', 'scope:shared', 'type:lib'],
+            },
+            // // define to which libraries ui can depend on
+            // { sourceTag: 'scope:ui', onlyDependOnLibsWithTags: ['scope:ui', 'scope:shared'] },
+          ],
         },
       ],
 
       // General style
       'prettier/prettier': 'error',
-      curly: ['error', 'all'],
 
       // TypeScript
       // Removed duplicates already enforced by TypeScript:
       'no-unused-vars': 'off',
+
+      // ts checks these better than ESLint
       'no-redeclare': 'off',
-      'no-dupe-class-members': 'off',
-      '@typescript-eslint/no-shadow': 'off',
+
+      /**
+       * This rule is redundant with `await` usage
+       * async function foo() {
+       *   return await bar(); // `return bar()` is sufficient
+       * }
+       *
+       * Also is covered by @no-floating-promises
+       */
       '@typescript-eslint/return-await': 'off',
 
+      /**
+       * This rule is too restrictive in practice
+       * const isAllow = useSomeHook(({ isAllow }) => isAllow); // will error
+       */
+      '@typescript-eslint/no-shadow': 'off',
+
+      // allow to type primitive types explicitly like `let x: number = 5;` ✅
+      '@typescript-eslint/no-inferrable-types': 'off',
+
+      /**
+       * this rule is too restrictive in practice, e.g.,
+       * expect(someAsyncFunction).toHaveBeenCalledWith(someArg); // will error because someAsyncFunction is async
+       */
+      '@typescript-eslint/no-misused-promises': 'off',
+
+      // allow the exception pattern with leading for function with multiple args
       '@typescript-eslint/no-unused-vars': [
         'error',
         { argsIgnorePattern: '^_', varsIgnorePattern: '^_' },
       ],
+
+      // functions an classes always are hoisted so this rule does not make sense for them
       '@typescript-eslint/no-use-before-define': [
         'error',
         { functions: false, classes: false, variables: true },
       ],
+
+      // warn on develop so is not super annoying but will be rejected by husky pre-commit
       '@typescript-eslint/no-explicit-any': 'warn',
 
+      // eslint cannot accurately check unbound methods so best to disable it
+      '@typescript-eslint/unbound-method': 'off',
+
+      /**
+       * Empty functions are useful as placeholders or in tests
+       */
+      '@typescript-eslint/no-empty-function': 'off',
+
+      // #region [TODO]: rules to review later
+      /**
+       * This rules are necessary in our code base but would require significant refactoring to enable them
+       * By now we disable them since they were muted by configuration or explicit rules... but we want them enabled eventually
+       */
+
+      /**
+       * Also necessary we should not render unknown values directly in templates
+       */
+      '@typescript-eslint/restrict-template-expressions': 'off',
+
+      /**
+       * Related with defensive programming
+       * We need to remove this rule and assert the non-null values properly: TODO
+       */
+      '@typescript-eslint/no-non-null-assertion': 'off',
+
+      /**
+       * This one is also related with defensive programming
+       * We need to assert every unknown value before using it: TODO
+       */
+      '@typescript-eslint/no-unsafe-call': 'off',
+
+      /**
+       * We need this rule to easier identify rejected promises origins
+       * It will require refactoring to enable it though
+       */
+      '@typescript-eslint/prefer-promise-reject-errors': 'off',
+
+      /**
+       * Avoid unsafe assignments
+       * We need to migrate to defensive programming and assert every unknown value before using it
+       */
+      '@typescript-eslint/no-unsafe-assignment': 'off',
+      '@typescript-eslint/no-unsafe-member-access': 'off',
+
+      /**
+       * Super important to avoid deadened async errors or forgotten promises,
+       * We already catch error because that could have been prevented by proper this rule usage
+       * It will require significant refactoring to enable it though
+       */
+      '@typescript-eslint/no-floating-promises': 'off',
+
+      /**
+       * Avoid creating class and interface with same name
+       * We need this but it requires refactoring to enable it
+       */
+      '@typescript-eslint/no-unsafe-declaration-merging': 'off',
+      'no-dupe-class-members': 'off',
+
+      // #endregion
+
       // React
       'react/react-in-jsx-scope': 'off',
       'react/jsx-uses-react': 'off',
+
       'react-hooks/rules-of-hooks': 'error',
       'react-hooks/exhaustive-deps': 'warn',
 

frontend/src/hooks/useDevices.tsx

@@ -38,7 +38,7 @@ const useDevices = () => {
       console.warn('enumerateDevices() not supported.');
       return;
     }
-    // eslint-disable-next-line consistent-return
+
     return new Promise<void>((_resolve, reject) => {
       // Vonage Video API's getDevices retrieves all audio and video input devices
       getDevices(async (err?: OTError, devices?: Device[]) => {

frontend/src/hooks/useLayoutManager.tsx

@@ -11,6 +11,7 @@ export type GetLayout = InstanceType<typeof LayoutManager>['getLayout'];
  */
 const useLayoutManager = (): GetLayout => {
   const layoutManager = useRef(new LayoutManager());
+  // eslint-disable-next-line react-hooks/refs
   return layoutManager.current.getLayout.bind(layoutManager.current);
 };
 

frontend/src/hooks/useRoomName.tsx

@@ -23,7 +23,7 @@ const useRoomName = ({ useLocationState = false }: UseRoomNameProps = {}): strin
     // this is the default behavior of this hook
     roomName = params.roomName;
   }
-  const lowerCaseRoomName = roomName?.toLowerCase() || '';
+  const lowerCaseRoomName: string = roomName?.toLowerCase() || '';
 
   return lowerCaseRoomName;
 };

frontend/src/hooks/useScreenShare.tsx

@@ -47,7 +47,7 @@ const useScreenShare = (): UseScreenShareType => {
     }
   }, [unpublish]);
 
-  const handleStreamCreated = useCallback(async () => {
+  const handleStreamCreated = useCallback(() => {
     unpublishScreenshare();
   }, [unpublishScreenshare]);
 
@@ -116,6 +116,10 @@ const useScreenShare = (): UseScreenShareType => {
     toggleShareScreen,
     isSharingScreen,
     screenshareVideoElement,
+    /**
+     * On the first render this will return null
+     */
+    // eslint-disable-next-line react-hooks/refs
     screensharingPublisher: screenSharingPub.current,
   };
 };

frontend/src/hooks/useToolbarButtons.tsx

@@ -107,7 +107,6 @@ const useToolbarButtons = ({
 
     observer.current.observe(toolbarRef.current);
 
-    // eslint-disable-next-line consistent-return
     return () => {
       if (observedToolbar) {
         observer.current?.unobserve(observedToolbar);

frontend/src/pages/MeetingRoom/MeetingRoom.spec.tsx

@@ -28,6 +28,7 @@ import usePublisherOptions from '@Context/PublisherProvider/usePublisherOptions'
 import { makeAppConfigProviderWrapper } from '@test/providers';
 import composeProviders from '@utils/composeProviders';
 import MeetingRoom from './MeetingRoom';
+import { Box } from 'opentok-layout-js';
 
 const mockedNavigate = vi.fn();
 const mockedParams = { roomName: 'test-room-name' };
@@ -165,7 +166,7 @@ describe('MeetingRoom', () => {
         left: 0,
         top: 0,
         width: 1280,
-      });
+      }) as Box[];
     });
     mockUseSessionContext.mockReturnValue(sessionContext as unknown as SessionContextType);
     mockUseActiveSpeaker.mockReturnValue(undefined);

frontend/src/pages/MeetingRoom/MeetingRoom.tsx

@@ -110,10 +110,8 @@ const MeetingRoom = (): ReactElement => {
     }
   }, [accessStatus]);
 
-  // eslint-disable-next-line @typescript-eslint/no-use-before-define
   useRedirectOnPublisherError(publishingError);
 
-  // eslint-disable-next-line @typescript-eslint/no-use-before-define
   useRedirectOnSubscriberError(subscriptionError);
 
   return (

frontend/src/pages/WaitingRoom/WaitingRoom.spec.tsx

@@ -149,6 +149,8 @@ describe('WaitingRoom', () => {
 
     const { rerender, container } = render(<WaitingRoomWithProviders />);
 
+    // TODO: investigate why this needs to be awaited or the test fails
+    // eslint-disable-next-line @typescript-eslint/await-thenable
     await act(() => {
       rerender(<WaitingRoomWithProviders />);
     });
@@ -182,7 +184,7 @@ describe('WaitingRoom', () => {
     expect(mockedDestroyPublisher).toHaveBeenCalled();
   });
 
-  it('should reload window when device permissions change', async () => {
+  it('should reload window when device permissions change', () => {
     const { rerender } = render(<WaitingRoomWithProviders />);
     expect(globalThis.location.reload).not.toBeCalled();
 
@@ -201,7 +203,7 @@ describe('WaitingRoom', () => {
  */
 function getLocationMock() {
   const { location } = globalThis;
-  const locationMock = Object.create(Object.getPrototypeOf(location));
+  const locationMock = Object.create(Object.getPrototypeOf(location) as Location);
 
   Object.assign(locationMock, location);
 

frontend/src/test/globals.ts

@@ -1,4 +1,3 @@
-/* eslint-disable import/prefer-default-export */
 // src/test-globals.ts
 export const setup = () => {
   process.env.TZ = 'US/Eastern';