Skip to content

fix: memory leak and layout shift issue #5119

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
krishhna24 wants to merge 1 commit into from
Closed

fix: memory leak and layout shift issue #5119

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 28 additions & 13 deletions pages/finance.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import Head from 'next/head';
import React, { useEffect, useRef, useState } from 'react';
import React, { useEffect, useState } from 'react';

import AsyncAPISummary from '../components/FinancialSummary/AsyncAPISummary';
import BarChartComponent from '../components/FinancialSummary/BarChartComponent';
Expand All @@ -16,21 +16,24 @@ import Container from '../components/layout/Container';
* bar chart, success stories, and contact us components.
*/
export default function FinancialSummary() {
const [windowWidth, setWindowWidth] = useState<number>(0);
// Initialize as null to avoid hydration/layout shift
const [windowWidth, setWindowWidth] = useState<number | null>(null);
const handleResize = () => {
console.log("resized")
setWindowWidth(window.innerWidth);
};
Comment on lines +19 to +24
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Feb 9, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Remove console.log, fix formatting, and move handleResize inside useEffect.

Multiple pipeline-failing issues here:

  1. console.log("resized") is a debug artifact — the linter rejects it (no-console).
  2. Double quotes must be single quotes (quotes rule).
  3. Indentation is inconsistent (extra leading space on line 21, extra indent on lines 23-24).
  4. Defining handleResize in the component body means a new reference is created each render, yet it's omitted from the useEffect deps. Move it inside the effect to keep the closure tight and avoid the stale-reference concern.
Proposed fix (move handler into useEffect) 
-  // Initialize as null to avoid hydration/layout shift
-  const [windowWidth, setWindowWidth] = useState<number | null>(null);
-   const handleResize = () => {
-    console.log("resized")
-      setWindowWidth(window.innerWidth);
-    };
-
-  // Properly scoped resize handler with correct cleanup
-  useEffect(() => {
-
-
-    // Set initial width on mount
-    handleResize();
-
-    window.addEventListener('resize', handleResize);
-
-    return () => {
-      window.removeEventListener('resize', handleResize);
-    };
-  }, []);
+  const [windowWidth, setWindowWidth] = useState<number | null>(null);
+
+  useEffect(() => {
+    const handleResize = () => {
+      setWindowWidth(window.innerWidth);
+    };
+
+    handleResize();
+    window.addEventListener('resize', handleResize);
+
+    return () => {
+      window.removeEventListener('resize', handleResize);
+    };
+  }, []);

Based on learnings: when intentionally omitting dependencies in React hooks, add an eslint-disable comment with an explanatory note. Here, moving the handler inside the effect eliminates the need for that.

🧰 Tools
🪛 GitHub Actions: PR testing - if Node project

[error] 21-21: Delete · prettier/prettier

[error] 22-22: Unexpected console statement. no-console

[error] 22-22: Replace "resized" ) with 'resized'); prettier/prettier

[error] 22-22: Strings must use singlequote. quotes

[error] 23-23: Delete ·· prettier/prettier

[error] 24-24: Delete ·· prettier/prettier

🤖 Prompt for AI Agents 
In `@pages/finance.tsx` around lines 19 - 24, The component currently defines
handleResize in the render body and includes a debug console.log and double
quotes plus bad indentation; move the handleResize function inside the useEffect
that subscribes to resizing, remove the console.log, use single quotes for any
strings (e.g., 'resized' if needed, though remove entirely here), and fix
indentation around the useState declaration and effect; update references to
windowWidth and setWindowWidth accordingly and ensure the effect adds/removes
the resize listener and lists no unnecessary dependencies (no eslint-disable
needed because the handler is declared inside useEffect).


// Properly scoped resize handler with correct cleanup
useEffect(() => {

const handleResizeRef = useRef<() => void>(null!);

handleResizeRef.current = () => {
setWindowWidth(window.innerWidth);
};
// Set initial width on mount
handleResize();

// Handle window resize event to update the window width state value for responsive design purposes
useEffect(() => {
handleResizeRef.current!();
window.addEventListener('resize', handleResizeRef.current!);
window.addEventListener('resize', handleResize);

return () => {
window.removeEventListener('resize', handleResizeRef.current!);
window.removeEventListener('resize', handleResize);
};
}, []);

Expand All @@ -41,8 +44,9 @@ export default function FinancialSummary() {
<>
<Head>
<title>{title}</title>
<meta name='description' content={description} />
<meta name="description" content={description} />
</Head>

<AsyncAPISummary />
<SponsorshipTiers />
<OtherFormsComponent />
Expand All @@ -53,7 +57,18 @@ export default function FinancialSummary() {
</>
);

// Avoid rendering until window size is known (prevents CLS)
if (windowWidth === null) {
return null; // or a skeleton/loading placeholder
}
Comment on lines +60 to +63
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Feb 9, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Returning null until client-side width is resolved hurts SSR/SEO and causes a flash of empty content.

Because windowWidth is null on the server (and on the first client render before the effect fires), this page will:

  • Send an empty HTML response to search engines / social-media crawlers.
  • Flash a blank screen before hydration completes.

This trades the original layout-shift problem for a blank-page problem. A better approach is to always render the content using a sensible default layout (e.g., the wide or narrow variant) and only switch once the width is known, or use CSS-only breakpoints for the layout difference (which the new JSX on lines 68-72 already does via 2xl: classes). If the layout is purely CSS-driven, you don't need the windowWidth guard at all.

Proposed fix — remove the null guard 
-  // Avoid rendering until window size is known (prevents CLS)
-  if (windowWidth === null) {
-    return null; // or a skeleton/loading placeholder
-  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Avoid rendering until window size is known (prevents CLS)
if (windowWidth === null) {
return null; // or a skeleton/loading placeholder
}
🤖 Prompt for AI Agents 
In `@pages/finance.tsx` around lines 60 - 63, The current guard returning null
when windowWidth === null causes blank SSR output; remove the early return and
always render the component (e.g., keep the exported Finance component JSX) and
rely on CSS-driven breakpoints (the existing 2xl: classes) or a sensible default
layout until the client effect sets windowWidth; update any conditional branches
that depended on windowWidth to fall back to a default (wide or narrow) when
windowWidth is null, and remove the null-check return to prevent empty HTML
being sent to crawlers.


const shouldUseContainer = windowWidth > 1700;
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Feb 9, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove unused shouldUseContainer — pipeline is failing on this.

This variable is computed but never referenced. If the layout is now CSS-driven (2xl: breakpoints), the JS-based width check is dead code. Remove it along with the windowWidth state and effect if they serve no other purpose.

Proposed fix 
-  const shouldUseContainer = windowWidth > 1700;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const shouldUseContainer = windowWidth > 1700;
🧰 Tools
🪛 GitHub Actions: PR testing - if Node project

[error] 65-65: ShouldUseContainer is assigned a value but never used. no-unused-vars

[error] 65-65: ShouldUseContainer is assigned a value but never used. @typescript-eslint/no-unused-vars

[error] 65-65: ShouldUseContainer is assigned a value but never used. unused-imports/no-unused-vars

🤖 Prompt for AI Agents 
In `@pages/finance.tsx` at line 65, Remove the unused JS-driven layout sizing:
delete the shouldUseContainer constant and the windowWidth state plus its
useEffect (and any window resize handler or cleanup) in pages/finance.tsx,
ensuring you also remove unused imports (useState/useEffect) if they become
unused; verify no other code references windowWidth or shouldUseContainer and
rely on the CSS breakpoints (e.g., 2xl:) instead.


return <div>{shouldUseContainer ? <Container wide>{renderComponents()}</Container> : renderComponents()}</div>;
return (
<div className="w-full">
<div className="2xl:container 2xl:mx-auto 2xl:px-8">
{renderComponents()}
</div>
</div>
);
Comment on lines +67 to +73
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Feb 9, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix indentation and quote style — multiple prettier/lint failures.

The JSX block uses double quotes (violating jsx-quotes) and inconsistent indentation (the inner <div> and children are not properly indented relative to the outer <div>).

Proposed fix 
-  return (
-    <div className="w-full">
-  <div className="2xl:container 2xl:mx-auto 2xl:px-8">
-    {renderComponents()}
-  </div>
-</div>
-  );
+  return (
+    <div className='w-full'>
+      <div className='2xl:container 2xl:mx-auto 2xl:px-8'>
+        {renderComponents()}
+      </div>
+    </div>
+  );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return (
<div className="w-full">
<div className="2xl:container 2xl:mx-auto 2xl:px-8">
{renderComponents()}
</div>
</div>
);
return (
<div className='w-full'>
<div className='2xl:container 2xl:mx-auto 2xl:px-8'>
{renderComponents()}
</div>
</div>
);
🧰 Tools
🪛 GitHub Actions: PR testing - if Node project

[error] 68-68: Replace "w-full" with 'w-full' prettier/prettier

[error] 68-68: Unexpected usage of doublequote. jsx-quotes

[error] 69-69: Replace <div className="2xl:container 2xl:mx-auto 2xl:px-8">\n····{renderComponents()}\n·· with <div className='2xl:container 2xl:mx-auto 2xl:px-8'>{renderComponents()} prettier/prettier

[error] 69-69: Unexpected usage of doublequote. jsx-quotes

[error] 72-72: Insert prettier/prettier

🤖 Prompt for AI Agents 
In `@pages/finance.tsx` around lines 67 - 73, The JSX return block in
pages/finance.tsx has inconsistent indentation and uses double quotes in JSX
attributes violating jsx-quotes; fix the indentation so the inner <div> and its
child {renderComponents()} are indented one level inside the outer <div>, and
change JSX attribute quotes from double to single quotes (e.g., className
attributes) to satisfy the linter; locate the return block containing
renderComponents() and adjust spacing and quote style accordingly.

}
Loading