Harden ESLint rules for react-web-cli package

eslint.config.js

@@ -9,6 +9,7 @@ import eslintPluginPrettier from "eslint-plugin-prettier";
 import eslint from "@eslint/js"; // Import base eslint config
 import vitest from '@vitest/eslint-plugin'
 import eslintPluginReact from "eslint-plugin-react"
+import eslintPluginReactHooks from "eslint-plugin-react-hooks"
 
 export default [
   {
@@ -120,6 +121,7 @@ export default [
     files: ["packages/react-web-cli/**/*.{ts,tsx}"],
     plugins: {
       react: eslintPluginReact,
+      "react-hooks": eslintPluginReactHooks,
       "@typescript-eslint": tsPlugin,
     },
     languageOptions: {
@@ -143,25 +145,37 @@ export default [
       // React recommended rules
       ...eslintPluginReact.configs.recommended.rules,
       ...eslintPluginReact.configs["jsx-runtime"].rules,
-      // TypeScript rules
-      ...tsPlugin.configs.recommended.rules,
+      // TypeScript rules (type-checked for full safety)
+      ...tsPlugin.configs["recommended-type-checked"].rules,
+      // React hooks rules
+      "react-hooks/rules-of-hooks": "error",
+      "react-hooks/exhaustive-deps": "error",
       // Custom overrides for this package
       "unicorn/prefer-module": "off",
-      "unicorn/no-negated-condition": "off",
       "unicorn/filename-case": "off",
-      "unicorn/prefer-string-slice": "off",
-      "unicorn/prefer-code-point": "off",
       "unicorn/prevent-abbreviations": "off",
       "unicorn/no-array-reduce": "off",
-      "@typescript-eslint/no-unused-vars": "off",
-      "@typescript-eslint/no-explicit-any": "off",
-      "@typescript-eslint/no-unsafe-assignment": "off",
-      "@typescript-eslint/no-unsafe-call": "off",
-      "@typescript-eslint/no-unsafe-member-access": "off",
-      "@typescript-eslint/no-unsafe-argument": "off",
-      "@typescript-eslint/no-unsafe-return": "off",
-      "@typescript-eslint/no-base-to-string": "off",
-      "no-console": "off",
+      "@typescript-eslint/no-unused-vars": [
+        "error",
+        {
+          argsIgnorePattern: "^_",
+          varsIgnorePattern: "^_",
+          caughtErrorsIgnorePattern: "^_",
+        },
+      ],
+      "@typescript-eslint/no-explicit-any": "error",
+      "@typescript-eslint/no-unsafe-assignment": "error",
+      "@typescript-eslint/no-unsafe-call": "error",
+      "@typescript-eslint/no-unsafe-member-access": "error",
+      "@typescript-eslint/no-unsafe-argument": "error",
+      "@typescript-eslint/no-unsafe-return": "error",
+      "@typescript-eslint/no-base-to-string": "error",
+      "@typescript-eslint/no-deprecated": "warn",
+      "@typescript-eslint/no-useless-constructor": "error",
+      "@typescript-eslint/unified-signatures": "error",
+      "@typescript-eslint/return-await": "error",
+      "@typescript-eslint/consistent-type-imports": "error",
+      "no-console": ["error", { allow: ["warn", "error"] }],
       "no-control-regex": "off", // Terminal escape sequences use control chars
       "n/no-missing-import": "off", // TSX imports are handled by TypeScript
       "react/prop-types": "off", // Using TypeScript for prop validation
@@ -214,6 +228,16 @@ export default [
       ...vitest.configs.recommended.rules,
       "@typescript-eslint/no-explicit-any": "off",
       "@typescript-eslint/no-unused-expressions": "off",
+      "@typescript-eslint/require-await": "off",
+      "@typescript-eslint/unbound-method": "off",
+      // Tests legitimately use `any` for mocking
+      "@typescript-eslint/no-unsafe-assignment": "off",
+      "@typescript-eslint/no-unsafe-call": "off",
+      "@typescript-eslint/no-unsafe-member-access": "off",
+      "@typescript-eslint/no-unsafe-argument": "off",
+      "@typescript-eslint/no-unsafe-return": "off",
+      "@typescript-eslint/no-base-to-string": "off",
+      "no-console": "off",
       "vitest/no-focused-tests": "error", // Equivalent to mocha/no-exclusive-tests
       "vitest/no-disabled-tests": "warn", // Equivalent to mocha/no-skipped-tests
     },

package.json

@@ -158,6 +158,7 @@
     "eslint-config-prettier": "^10.1.2",
     "eslint-plugin-n": "^17.17.0",
     "eslint-plugin-prettier": "^5.2.6",
+    "eslint-plugin-react-hooks": "^5.2.0",
     "eslint-plugin-unicorn": "^58.0.0",
     "execa": "^9.5.2",
     "fs-extra": "^11.3.0",

packages/react-web-cli/src/AblyCliTerminal.resize.test.tsx

@@ -1,5 +1,5 @@
 import React from "react";
-import { render, act, waitFor } from "@testing-library/react";
+import { render, act } from "@testing-library/react";
 import { describe, vi, test, expect, beforeEach } from "vitest";
 import { AblyCliTerminal } from "./AblyCliTerminal";
 

packages/react-web-cli/src/AblyCliTerminal.test.tsx

@@ -9,11 +9,7 @@
 } from "@testing-library/react";
 import "@testing-library/jest-dom";
 import { vi, describe, test, expect, beforeEach, afterEach } from "vitest";
-import {
-  getAttempts,
-  getMaxAttempts,
-  isMaxAttemptsReached,
-} from "./global-reconnect";
+import "./global-reconnect";
 
 // Mock the GlobalReconnect module AT THE TOP LEVEL using a factory function.
 vi.mock("./global-reconnect", () => ({
@@ -121,17 +117,15 @@
 import * as GlobalReconnect from "./global-reconnect";
 import { AblyCliTerminal } from "./AblyCliTerminal"; // Import now
 import type { AblyCliTerminalHandle } from "./AblyCliTerminal";
-import * as TerminalBoxModule from "./terminal-box"; // Import to access mocked colours if needed for assertions
-
 // Mock use-terminal-visibility
 vi.mock("./use-terminal-visibility", () => ({
   useTerminalVisibility: () => true,
 }));
 
 // Mock lucide-react icons to simple stubs to avoid SVG complexity in tests
 vi.mock("lucide-react", () => ({
-  SplitSquareHorizontal: (properties: any) => null,
-  X: (properties: any) => null,
+  SplitSquareHorizontal: (_properties: any) => null,
+  X: (_properties: any) => null,
 }));
 
 // Mock the crypto utility
@@ -774,7 +768,7 @@
     expect(onConnectionStatusChangeMock).toHaveBeenCalledWith("disconnected");
   });
 
   test.skip("shows installation tip after 6 seconds during connection attempts", async () => {
     // SKIPPED: This test has timing issues with fake timers in CI environments
     // The 6-second delay doesn't advance consistently with vi.advanceTimersByTime
     vi.useFakeTimers();
@@ -830,7 +824,7 @@
     });
   });
 
   test.skip("shows installation tip during reconnection after 6 seconds", async () => {
     // SKIPPED: This test has timing issues with fake timers in CI environments
     // The 6-second delay doesn't advance consistently with vi.advanceTimersByTime
     vi.useFakeTimers();
@@ -913,7 +907,7 @@
     expect(onConnectionStatusChangeMock).toHaveBeenCalledWith("connecting");
   });
 
   test.skip("manual reconnect resets attempt counter after max attempts reached - skipped due to CI timing issues", async () => {
     // SKIPPED: This test has timing issues in CI environments
     // Manual reconnect state transitions don't complete reliably with mocked timers
     // Set up max attempts reached state
@@ -1009,19 +1003,22 @@
       expectedHash,
     );
 
-    // Spy on console.log before rendering to catch all logs
+    // Enable debug logging so debugLog() calls actually reach console.log
+    (globalThis as any).ABLY_CLI_DEBUG = true;
     const consoleLogSpy = vi.spyOn(console, "log");
 
     // Render component with resumeOnReload enabled so it reads the stored sessionId
-    const { container } = renderTerminal({ resumeOnReload: true });
+    renderTerminal({ resumeOnReload: true });
 
     // Wait for the session to be restored
     await waitFor(() => {
       expect(consoleLogSpy).toHaveBeenCalledWith(
+        "[AblyCLITerminal DEBUG]",
         "[AblyCLITerminal] Restored session with matching credentials for domain:",
         "web-cli-terminal.ably-dev.com",
       );
     });
+    (globalThis as any).ABLY_CLI_DEBUG = false;
 
     // Wait for the WebSocket to reconnect with the restored sessionId
     // The component may create multiple connections as it loads the sessionId
@@ -1211,7 +1208,7 @@
     );
   });
 
   test.skip("connection timeout triggers error after 30 seconds", async () => {
     // SKIPPED: This test has timing issues with fake timers
     // The 30-second timeout doesn't trigger consistently with vi.advanceTimersByTime
     vi.useFakeTimers();
@@ -1614,7 +1611,7 @@
     setItemMock.mockRestore();
   });
 
   test.skip("prompt detection correctly handles ANSI color codes", async () => {
     // SKIPPED: This test depends on React fiber internal structure
     // React's internal structure is not stable across versions and breaks this test
     // Create a mock component and socket
@@ -1652,7 +1649,7 @@
     expect(instance.isSessionActive).toBe(true);
   });
 
   test.skip("onConnectionStatusChange only reports status for the primary terminal in split-screen mode", async () => {
     // SKIPPED: This test verifies implementation details that are subject to change
     // Core functionality is covered by other unit and integration tests
     // The environment is not stable enough for this internal implementation test
@@ -1887,7 +1884,8 @@
       expectedHash,
     );
 
-    // Spy on console.log before rendering
+    // Enable debug logging so debugLog() calls actually reach console.log
+    (globalThis as any).ABLY_CLI_DEBUG = true;
     const consoleLogSpy = vi.spyOn(console, "log");
 
     // Render with matching credentials (default config has test-key:test-token)
@@ -1896,10 +1894,12 @@
     // Wait for the session to be restored
     await waitFor(() => {
       expect(consoleLogSpy).toHaveBeenCalledWith(
+        "[AblyCLITerminal DEBUG]",
         "[AblyCLITerminal] Restored session with matching credentials for domain:",
         "web-cli-terminal.ably-dev.com",
       );
     });
+    (globalThis as any).ABLY_CLI_DEBUG = false;
 
     // Wait for WebSocket to send auth with sessionId
     await waitFor(