Merge pull request #2 from ragusa-it/fix-typing-effect-cleanup-379426751446782624

Fix memory leak in useTypingEffect hook

package.json

@@ -24,10 +24,14 @@
     "react-router-dom": "^7.12.0"
   },
   "devDependencies": {
+    "@testing-library/dom": "^10.4.1",
+    "@testing-library/react": "^16.3.2",
     "@types/react": "^19.2.9",
     "@types/react-dom": "^19.2.3",
     "@vitejs/plugin-react": "^5.1.2",
+    "jsdom": "^27.4.0",
     "typescript": "^5.9.3",
-    "vite": "^7.3.1"
+    "vite": "^7.3.1",
+    "vitest": "^4.0.17"
   }
 }

src/hooks/useTypingEffect.test.ts

@@ -0,0 +1,109 @@
+import { renderHook, act } from '@testing-library/react';
+import { useTypingEffect } from './useTypingEffect';
+import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest';
+
+describe('useTypingEffect', () => {
+  beforeEach(() => {
+    vi.useFakeTimers();
+  });
+
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
+
+  it('should type text, pause, and delete', async () => {
+    const { result } = renderHook(() =>
+      useTypingEffect({
+        words: ['Hello'],
+        typingSpeed: 10,
+        deletingSpeed: 10,
+        pauseDuration: 100,
+      })
+    );
+
+    // Initial state
+    expect(result.current.text).toBe('');
+
+    // Advance time incrementally to ensure state updates and effects run
+    // "Hello" is 5 chars. Should take roughly 50-60ms.
+    // We loop more than enough times to ensure it completes.
+    for (let i = 0; i < 10; i++) {
+        await act(async () => {
+            vi.advanceTimersByTime(11);
+        });
+        if (result.current.text === 'Hello') break;
+    }
+
+    expect(result.current.text).toBe('Hello');
+
+    // Should be in pause state (isTyping false, isDeleting false)
+    // Wait for pause duration (100ms)
+    await act(async () => {
+        vi.advanceTimersByTime(50);
+    });
+    // Check if paused
+    expect(result.current.isTyping).toBe(false);
+    expect(result.current.isDeleting).toBe(false);
+
+    await act(async () => {
+        vi.advanceTimersByTime(60);
+    });
+
+    // Should be deleting now
+    expect(result.current.isDeleting).toBe(true);
+
+    // Advance to delete "Hello"
+    for (let i = 0; i < 10; i++) {
+        await act(async () => {
+            vi.advanceTimersByTime(11);
+        });
+        if (result.current.text === '') break;
+    }
+
+    expect(result.current.text).toBe('');
+    expect(result.current.isDeleting).toBe(false);
+  });
+
+  it('should clean up timers on unmount', async () => {
+    const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+
+    const { result, unmount } = renderHook(() =>
+      useTypingEffect({
+        words: ['Hi'],
+        typingSpeed: 10,
+        deletingSpeed: 10,
+        pauseDuration: 100,
+      })
+    );
+
+    // Type "Hi"
+    for (let i = 0; i < 10; i++) {
+        await act(async () => {
+            vi.advanceTimersByTime(11);
+        });
+        if (result.current.text === 'Hi') break;
+    }
+    expect(result.current.text).toBe('Hi');
+
+    // Trigger the pause logic
+    await act(async () => {
+        vi.advanceTimersByTime(20);
+    });
+
+    // Unmount the component
+    unmount();
+
+    // Advance time past pause where the timeout would fire
+    await act(async () => {
+        vi.advanceTimersByTime(200);
+    });
+
+    // We verify no console errors regarding unmounted updates
+    const unmountErrors = consoleSpy.mock.calls.filter(args =>
+      args[0] && typeof args[0] === 'string' &&
+      (args[0].includes('unmounted component') || args[0].includes('state update'))
+    );
+
+    expect(unmountErrors).toHaveLength(0);
+  });
+});

src/hooks/useTypingEffect.ts

@@ -1,4 +1,4 @@
-import { useState, useEffect, useCallback } from 'react';
+import { useState, useEffect } from 'react';
 
 interface UseTypingEffectOptions {
   words: string[];
@@ -18,38 +18,50 @@ export function useTypingEffect({
   const [isDeleting, setIsDeleting] = useState(false);
   const [isPaused, setIsPaused] = useState(false);
 
-  const tick = useCallback(() => {
-    const currentWord = words[currentWordIndex];
+  useEffect(() => {
+    let timer: ReturnType<typeof setTimeout>;
 
     if (isPaused) {
-      return;
-    }
+      timer = setTimeout(() => {
+        setIsPaused(false);
+        setIsDeleting(true);
+      }, pauseDuration);
+    } else {
+      const currentWord = words[currentWordIndex];
+      const isWordDeleted = isDeleting && currentText === '';
 
-    if (isDeleting) {
-      setCurrentText(currentWord.substring(0, currentText.length - 1));
-      
-      if (currentText.length === 0) {
+      if (isWordDeleted) {
         setIsDeleting(false);
         setCurrentWordIndex((prev) => (prev + 1) % words.length);
-      }
-    } else {
-      setCurrentText(currentWord.substring(0, currentText.length + 1));
-      
-      if (currentText === currentWord) {
-        setIsPaused(true);
-        setTimeout(() => {
-          setIsPaused(false);
-          setIsDeleting(true);
-        }, pauseDuration);
+      } else {
+        const speed = isDeleting ? deletingSpeed : typingSpeed;
+
+        timer = setTimeout(() => {
+          if (isDeleting) {
+            setCurrentText((prev) => prev.substring(0, prev.length - 1));
+          } else {
+            const nextText = currentWord.substring(0, currentText.length + 1);
+            setCurrentText(nextText);
+
+            if (nextText === currentWord) {
+              setIsPaused(true);
+            }
+          }
+        }, speed);
       }
     }
-  }, [currentText, currentWordIndex, isDeleting, isPaused, words, pauseDuration]);
 
-  useEffect(() => {
-    const speed = isDeleting ? deletingSpeed : typingSpeed;
-    const timer = setTimeout(tick, speed);
     return () => clearTimeout(timer);
-  }, [tick, isDeleting, typingSpeed, deletingSpeed]);
+  }, [
+    currentText,
+    isDeleting,
+    isPaused,
+    currentWordIndex,
+    words,
+    typingSpeed,
+    deletingSpeed,
+    pauseDuration,
+  ]);
 
   return {
     text: currentText,