Skip to content

chore: Upgrade storybook#288

Merged
ryan953 merged 3 commits intomainfrom
ryan953/deps-storybook
Nov 12, 2025
Merged

chore: Upgrade storybook#288
ryan953 merged 3 commits intomainfrom
ryan953/deps-storybook

Conversation

@ryan953
Copy link
Copy Markdown
Member

@ryan953 ryan953 commented Nov 12, 2025

No description provided.

@ryan953 ryan953 changed the title chore: Upgrade storybook to v9 chore: Upgrade storybook Nov 12, 2025
@ryan953 ryan953 marked this pull request as ready for review November 12, 2025 06:36
@ryan953 ryan953 requested a review from a team as a code owner November 12, 2025 06:36
Comment thread .storybook/preview.tsx
Comment on lines +1 to 4
import type {Preview} from '@storybook/react-vite';
import '../src/lib/index.css';
import Providers from 'toolbar/context/Providers';
import type {Configuration} from 'toolbar/types/Configuration';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: useEffect in Providers.tsx calls setColorScheme but fails to return its cleanup function, leading to unmanaged event listener accumulation and memory leaks.
Severity: HIGH | Confidence: 0.95

🔍 Detailed Analysis

The setColorScheme function, which returns a cleanup function for event listeners, is called within useEffect hooks in Providers.tsx. However, these useEffect hooks fail to return the cleanup function. This omission prevents the removal of old event listeners when config.theme or reactMount changes, or when the component unmounts. Consequently, event listeners accumulate, leading to a memory leak and potential performance degradation over time.

💡 Suggested Fix

Modify the useEffect hooks in Providers.tsx to return the cleanup function provided by setColorScheme. For example, useEffect(() => { const cleanup = setColorScheme(reactMount, config.theme); return cleanup; }, [config.theme, reactMount]);

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: .storybook/preview.tsx#L1-L4

Potential issue: The `setColorScheme` function, which returns a cleanup function for
event listeners, is called within `useEffect` hooks in `Providers.tsx`. However, these
`useEffect` hooks fail to return the cleanup function. This omission prevents the
removal of old event listeners when `config.theme` or `reactMount` changes, or when the
component unmounts. Consequently, event listeners accumulate, leading to a memory leak
and potential performance degradation over time.

Did we get this right? 👍 / 👎 to inform future reviews.

@ryan953 ryan953 merged commit a7fd1b7 into main Nov 12, 2025
7 checks passed
@ryan953 ryan953 deleted the ryan953/deps-storybook branch November 12, 2025 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant