Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .jules/bolt.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,8 @@

**Learning:** Found `useEffect` fetching static content (Speakers) in a Client Component (`Section5`) on the homepage. This caused unnecessary layout shifts and delayed LCP.
**Action:** Move data fetching to the parent Server Component (`page.tsx`) and pass data as props. This leverages ISR caching and eliminates client-side waterfall.

## 2026-03-17 - Scroll Event Listener Anti-Pattern

**Learning:** Found multiple instances of `scroll` event listeners attached to the `window` or `document` object without debouncing or throttling. These events can trigger frequently and block the main thread.
**Action:** When adding scroll event listeners, use `{ passive: true }` options and throttle execution using `requestAnimationFrame` to prevent main-thread blocking. Ensure local state variables used for throttling are wrapped in a constant object to satisfy ESLint `no-restricted-syntax`.
7 changes: 7 additions & 0 deletions __tests__/components/layout/DynamicHeaderWrapper.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,16 @@ describe("DynamicHeaderWrapper", () => {

expect(screen.getByTestId("header8")).toHaveAttribute("data-scroll", "false");

const rafMock = jest.spyOn(window, "requestAnimationFrame").mockImplementation((cb) => {
cb(0);
return 1;
});

Object.defineProperty(window, "scrollY", { value: 120, writable: true, configurable: true });
fireEvent.scroll(document);

expect(screen.getByTestId("header8")).toHaveAttribute("data-scroll", "true");

rafMock.mockRestore();
});
});
7 changes: 7 additions & 0 deletions __tests__/components/layout/Layout.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,16 @@ describe("Layout", () => {

expect(screen.getByTestId("header-1")).toHaveAttribute("data-scroll", "false");

const rafMock = jest.spyOn(window, "requestAnimationFrame").mockImplementation((cb) => {
cb(0);
return 1;
});

Object.defineProperty(window, "scrollY", { value: 150, writable: true, configurable: true });
fireEvent.scroll(document);

expect(screen.getByTestId("header-1")).toHaveAttribute("data-scroll", "true");

rafMock.mockRestore();
});
});
12 changes: 10 additions & 2 deletions components/elements/BackToTop.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,19 @@ export default function BackToTop({ target }: Readonly<{ target: string }>) {
const [hasScrolled, setHasScrolled] = useState(false);

useEffect(() => {
const state = { isTicking: false };

const onScroll = () => {
setHasScrolled(window.scrollY > 100);
if (!state.isTicking) {
window.requestAnimationFrame(() => {
setHasScrolled(window.scrollY > 100);
state.isTicking = false;
});
state.isTicking = true;
}
};

window.addEventListener("scroll", onScroll);
window.addEventListener("scroll", onScroll, { passive: true });
return () => window.removeEventListener("scroll", onScroll);
}, []);

Expand Down
16 changes: 12 additions & 4 deletions components/layout/DynamicHeaderWrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,21 @@ export default function DynamicHeaderWrapper({ navigation }: Readonly<DynamicHea
const [scroll, setScroll] = useState<boolean>(false);

React.useEffect(() => {
const state = { isTicking: false };

const handleScroll = (): void => {
const scrollCheck: boolean = window.scrollY > 100;
if (scrollCheck !== scroll) {
setScroll(scrollCheck);
if (!state.isTicking) {
window.requestAnimationFrame(() => {
const scrollCheck: boolean = window.scrollY > 100;
if (scrollCheck !== scroll) {
setScroll(scrollCheck);
}
Comment on lines +24 to +27
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The useEffect hook for this component has scroll in its dependency array (on line 37), causing the event listener to be re-attached every time the scroll state changes. This is inefficient and is caused by the dependency on scroll within this if condition.

To improve this, you can remove the conditional check and call setScroll directly. React is already optimized to avoid re-renders if the state value is the same. This change will allow you to remove scroll from the useEffect dependency array (changing it to []), ensuring the listener is attached only once.

The implementation in components/elements/BackToTop.tsx is a good example of this pattern.

Suggested change
const scrollCheck: boolean = window.scrollY > 100;
if (scrollCheck !== scroll) {
setScroll(scrollCheck);
}
setScroll(window.scrollY > 100);

state.isTicking = false;
});
state.isTicking = true;
}
};
document.addEventListener("scroll", handleScroll);
document.addEventListener("scroll", handleScroll, { passive: true });
return () => {
document.removeEventListener("scroll", handleScroll);
};
Expand Down
16 changes: 12 additions & 4 deletions components/layout/Layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,22 @@

useEffect(() => {
AOS.init();
const state = { isTicking: false };

const handleScroll = (): void => {
const scrollCheck: boolean = window.scrollY > 100;
if (scrollCheck !== scroll) {
setScroll(scrollCheck);
if (!state.isTicking) {
window.requestAnimationFrame(() => {
const scrollCheck: boolean = window.scrollY > 100;
if (scrollCheck !== scroll) {
setScroll(scrollCheck);
}
Comment on lines +87 to +90
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Similar to DynamicHeaderWrapper.tsx, the useEffect hook has scroll in its dependency array (on line 102), which is inefficient as it causes the event listener to be re-attached on every scroll state change. This is caused by the if (scrollCheck !== scroll) check.

To optimize this, you can remove the if check and let React handle state update optimizations. This will allow you to use an empty dependency array [] for the useEffect, ensuring the listener is attached only once. After applying the suggestion below, please also change the dependency array on line 102 from [scroll] to [].

Suggested change
const scrollCheck: boolean = window.scrollY > 100;
if (scrollCheck !== scroll) {
setScroll(scrollCheck);
}
setScroll(window.scrollY > 100);

state.isTicking = false;
});
state.isTicking = true;
}
};

document.addEventListener("scroll", handleScroll);
document.addEventListener("scroll", handleScroll, { passive: true });

return () => {
document.removeEventListener("scroll", handleScroll);
Expand All @@ -101,9 +109,9 @@

const resolvedHeaderStyle = headerStyle || 1;
const resolvedFooterStyle = footerStyle || 1;
const headerRenderer = headerRenderers[resolvedHeaderStyle] ?? headerRenderers[1];

Check warning on line 112 in components/layout/Layout.tsx

View workflow job for this annotation

GitHub Actions / lint

Generic Object Injection Sink
const headerElement = headerRenderer({ scroll, isSearch, handleSearch }, defaultNavigation);
const footerElement = footerComponents[resolvedFooterStyle] ?? footerComponents[1];

Check warning on line 114 in components/layout/Layout.tsx

View workflow job for this annotation

GitHub Actions / lint

Generic Object Injection Sink

return (
<>
Expand Down
Loading