Skip to content

replace useState with useEffect in useBreakpoint hook to prevent memory leak#851

Closed
divyansh-cyber wants to merge 1 commit intoaccordproject:mainfrom
divyansh-cyber:fix/useBreakpoint-memory-leak
Closed

replace useState with useEffect in useBreakpoint hook to prevent memory leak#851
divyansh-cyber wants to merge 1 commit intoaccordproject:mainfrom
divyansh-cyber:fix/useBreakpoint-memory-leak

Conversation

@divyansh-cyber
Copy link
Copy Markdown

Fixes #850

Description

The custom useBreakpoint hook in Navbar.tsx was incorrectly using useState to register a window.addEventListener('resize', ...) side-effect. React's useState initializer runs once but silently discards any return value, meaning the cleanup function (removeEventListener) was never called.

This caused:

  • Memory leak: Orphaned resize listeners accumulated on every component remount (e.g., route navigation).
  • React Strict Mode issues: In development, Strict Mode double-mounts components, doubling the leaked listeners with no cleanup.

Changes Made

File Change
src/components/Navbar.tsx Replaced useState with useEffect (+ empty [] deps) in the useBreakpoint hook

Before (Bug)

useState(() => {
  // ...
  window.addEventListener('resize', checkSize);
  return () => window.removeEventListener('resize', checkSize); // ← never called
});

After (Fix)

useEffect(() => {
  // ...
  window.addEventListener('resize', checkSize);
  return () => window.removeEventListener('resize', checkSize); // ← properly cleaned up
}, []);

How to Test

  1. Open the app and navigate between / and /learn/intro multiple times.
  2. Open DevTools → Performance Monitor → check "Event Listeners" count.
  3. Before fix: listener count increases with each navigation.
  4. After fix: listener count stays stable.

Checklist

  • Code follows the project's style guidelines
  • No new dependencies added
  • Change is backward-compatible
  • Tested locally responsive Navbar behavior works as expected

…ry leak

Signed-off-by: Divyansh Rai <140232173+divyansh-cyber@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 23, 2026 12:24
@divyansh-cyber divyansh-cyber requested a review from a team as a code owner March 23, 2026 12:24
@netlify
Copy link
Copy Markdown

netlify bot commented Mar 23, 2026

Deploy Preview for ap-template-playground ready!

Name Link
🔨 Latest commit c295876
🔍 Latest deploy log https://app.netlify.com/projects/ap-template-playground/deploys/69c130f097d19f000848633a
😎 Deploy Preview https://deploy-preview-851--ap-template-playground.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a lifecycle bug in the custom useBreakpoint hook used by Navbar by moving the resize event listener setup/cleanup into useEffect, preventing leaked listeners across remounts (notably under React Strict Mode).

Changes:

  • Import useEffect and replace the prior useState initializer side-effect with a useEffect(..., []) subscription.
  • Ensure the resize listener is properly removed on unmount via the effect cleanup.

Comment thread src/components/Navbar.tsx
Comment on lines 167 to +188
const useBreakpoint = () => {
const [screenSize, setScreenSize] = useState({
sm: false,
md: false,
lg: false,
xl: false,
});

useState(() => {
useEffect(() => {
const checkSize = () => {
setScreenSize({
sm: window.innerWidth >= 640,
md: window.innerWidth >= 768,
lg: window.innerWidth >= 1024,
xl: window.innerWidth >= 1280,
});
};

checkSize();
window.addEventListener('resize', checkSize);
return () => window.removeEventListener('resize', checkSize);
});
}, []);
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useBreakpoint now initializes screenSize to all false and only updates it in useEffect, which means the navbar can briefly render the “mobile” layout on desktop before the effect runs (layout shift/flash). Consider initializing state from window.innerWidth in the useState initializer (or using useLayoutEffect) so the first render matches the current viewport, and keep the effect only for the resize subscription/cleanup.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

This PR is stale because it has been open 15 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Apr 8, 2026
@github-actions github-actions bot closed this Apr 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect use of useState instead of useEffect in useBreakpoint

2 participants