Replace React Context with Zustand for form state management#29
Conversation
Introduces zustand as the state management library, replacing the FormContext/Provider pattern. The existing hooks remain the source of truth while App.tsx syncs their values into the zustand store. Consumer components now use useFormStore() instead of useFormContext(), gaining fine-grained subscriptions without needing a Provider wrapper. - Add zustand dependency - Create src/store/useFormStore.ts as the central form store - Remove FormContext, createContext/useContext, and Provider from App.tsx - Update all consumers (SearchForm, SettingsDialog, EventView, IssuesAndPRsList, Summary) to import from useFormStore - Update test mocks and test-utils to use zustand store - Remove unused FormContextType and ResultsContextType from types.ts https://claude.ai/code/session_01BM8QfufDNGMCvJPVBEAw5w
There was a problem hiding this comment.
Pull request overview
This PR migrates form state management from React Context to Zustand, eliminating the need for a Context Provider wrapper while maintaining functionality. The implementation uses a hybrid approach where existing hooks in App.tsx remain the source of truth, and a useEffect hook syncs their values to a Zustand store that child components subscribe to.
Changes:
- Created a new Zustand store (
useFormStore) to replaceFormContextanduseFormContext - Added synchronization logic in App.tsx to push hook state into the Zustand store via
useEffect - Updated all consumer components and tests to use
useFormStoreinstead ofuseFormContext
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/store/useFormStore.ts | New Zustand store with form state and actions; replaces FormContext |
| src/App.tsx | Removed Context provider wrapper and added useEffect to sync state to Zustand store |
| src/components/SearchForm.tsx | Updated to import and use useFormStore instead of useFormContext |
| src/components/SettingsDialog.tsx | Updated to import and use useFormStore instead of useFormContext |
| src/views/EventView.tsx | Updated to import and use useFormStore instead of useFormContext |
| src/views/IssuesAndPRsList.tsx | Updated to import and use useFormStore instead of useFormContext |
| src/views/Summary.tsx | Updated to import and use useFormStore instead of useFormContext |
| src/types.ts | Removed FormContextType and ResultsContextType (now in store file) |
| src/test/setup.ts | Replaced context mocking with direct Zustand store seeding |
| src/test/test-utils.tsx | Removed context providers and added seedFormStore helper |
| src/components/tests/Summary.test.tsx | Updated mock to use useFormStore instead of useFormContext |
| src/components/tests/SearchForm-RaceCondition.test.tsx | Updated mock to use useFormStore instead of useFormContext |
| src/views/tests/IssuesAndPRsList.test.tsx | Updated mock to use useFormStore instead of useFormContext |
| src/components/SearchForm.test.tsx | Updated mock to use useFormStore instead of useFormContext |
| package.json | Added zustand@^5.0.11 dependency |
| package-lock.json | Added zustand dependency with metadata |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| useFormStore.setState({ | ||
| username, startDate, endDate, githubToken, apiMode, searchText, | ||
| loading, loadingProgress, error, | ||
| searchItemsCount, eventsCount, rawEventsCount, | ||
| setUsername, setStartDate, setEndDate, setGithubToken, setApiMode, setSearchText, | ||
| handleSearch, validateUsernameFormat, addAvatarsToCache, | ||
| }); | ||
| }, [ | ||
| username, startDate, endDate, githubToken, apiMode, searchText, | ||
| loading, loadingProgress, error, | ||
| searchItemsCount, eventsCount, rawEventsCount, | ||
| setUsername, setStartDate, setEndDate, setGithubToken, setApiMode, setSearchText, | ||
| handleSearch, validateUsernameFormat, addAvatarsToCache, |
There was a problem hiding this comment.
The useEffect hook that syncs state to the Zustand store runs on every change to any of the dependencies (lines 121-127). This approach creates a potential performance issue because every state change triggers the entire store to be updated via setState, which will notify all subscribers even if they're only interested in a single field. While Zustand does optimize re-renders through selectors, this synchronization pattern negates some of those benefits. Consider whether this hybrid approach (hooks as source of truth + Zustand for distribution) is necessary, or if migrating fully to Zustand would be more performant and maintainable.
| useFormStore.setState({ | |
| username, startDate, endDate, githubToken, apiMode, searchText, | |
| loading, loadingProgress, error, | |
| searchItemsCount, eventsCount, rawEventsCount, | |
| setUsername, setStartDate, setEndDate, setGithubToken, setApiMode, setSearchText, | |
| handleSearch, validateUsernameFormat, addAvatarsToCache, | |
| }); | |
| }, [ | |
| username, startDate, endDate, githubToken, apiMode, searchText, | |
| loading, loadingProgress, error, | |
| searchItemsCount, eventsCount, rawEventsCount, | |
| setUsername, setStartDate, setEndDate, setGithubToken, setApiMode, setSearchText, | |
| handleSearch, validateUsernameFormat, addAvatarsToCache, | |
| const currentState = useFormStore.getState() as any; | |
| const nextState = { | |
| username, | |
| startDate, | |
| endDate, | |
| githubToken, | |
| apiMode, | |
| searchText, | |
| loading, | |
| loadingProgress, | |
| error, | |
| searchItemsCount, | |
| eventsCount, | |
| rawEventsCount, | |
| setUsername, | |
| setStartDate, | |
| setEndDate, | |
| setGithubToken, | |
| setApiMode, | |
| setSearchText, | |
| handleSearch, | |
| validateUsernameFormat, | |
| addAvatarsToCache, | |
| }; | |
| const changedState: Partial<typeof nextState> = {}; | |
| let hasChanges = false; | |
| (Object.keys(nextState) as (keyof typeof nextState)[]).forEach((key) => { | |
| if (currentState[key] !== nextState[key]) { | |
| changedState[key] = nextState[key]; | |
| hasChanges = true; | |
| } | |
| }); | |
| if (hasChanges) { | |
| useFormStore.setState(changedState); | |
| } | |
| }, [ | |
| username, | |
| startDate, | |
| endDate, | |
| githubToken, | |
| apiMode, | |
| searchText, | |
| loading, | |
| loadingProgress, | |
| error, | |
| searchItemsCount, | |
| eventsCount, | |
| rawEventsCount, | |
| setUsername, | |
| setStartDate, | |
| setEndDate, | |
| setGithubToken, | |
| setApiMode, | |
| setSearchText, | |
| handleSearch, | |
| validateUsernameFormat, | |
| addAvatarsToCache, |
| setGithubToken: (token: string) => void; | ||
| setApiMode: (mode: ApiMode) => void; | ||
| setSearchText: (text: string) => void; | ||
| handleSearch: () => void; |
There was a problem hiding this comment.
The type signature for handleSearch is incorrect. The actual implementation in useGitHubDataFetching returns Promise<void> (line 25 in useGitHubDataFetching.ts), but this type signature specifies () => void. This type mismatch can cause TypeScript type checking issues and potentially hide bugs where the promise is not handled correctly. The signature should be handleSearch: () => Promise<void>; to match the actual implementation.
| * State values and action callbacks are synced from App.tsx via | ||
| * `useFormStoreSync` so that the existing hooks remain the source of truth | ||
| * while consumers get fine-grained subscriptions without a Provider. |
There was a problem hiding this comment.
The documentation references useFormStoreSync which doesn't exist. The actual synchronization happens via a useEffect hook in App.tsx (lines 113-127). This should be updated to accurately reflect the implementation, perhaps: "State values and action callbacks are synced from App.tsx via a useEffect hook..."
| * State values and action callbacks are synced from App.tsx via | |
| * `useFormStoreSync` so that the existing hooks remain the source of truth | |
| * while consumers get fine-grained subscriptions without a Provider. | |
| * State values and action callbacks are synced from App.tsx via a `useEffect` | |
| * hook so that the existing hooks remain the source of truth while consumers | |
| * get fine-grained subscriptions without a Provider. |
| @@ -26,9 +26,8 @@ const mockFormContext = { | |||
| rawEventsCount: 0, | |||
| }; | |||
There was a problem hiding this comment.
The mock is missing required fields that are used by the Summary component. The searchText, setSearchText, and addAvatarsToCache fields are used in the component (line 75 in Summary.tsx) but are not included in this mock. This could cause the test to fail or behave unexpectedly. These fields should be added to match the complete FormStore interface.
|
@copilot open a new pull request to apply changes based on the comments in this thread |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const { searchText, setSearchText } = useFormStore(); | ||
|
|
There was a problem hiding this comment.
Using useFormStore() without a selector subscribes this component to the entire store, so any store update will re-render this (potentially heavy) list. To get the fine-grained subscription benefits, select only the needed slices (e.g., searchText and setSearchText), optionally using Zustand's shallow helper when selecting multiple values.
| rawEvents = [], | ||
| }: EventViewProps) { | ||
| const { searchText, setSearchText } = useFormContext(); | ||
| const { searchText, setSearchText } = useFormStore(); |
There was a problem hiding this comment.
Using useFormStore() without a selector subscribes this view to the entire store, so unrelated updates (counts, loading, etc.) will re-render the whole list. Switch to selecting only searchText / setSearchText (and use shallow if selecting multiple fields) to avoid unnecessary renders.
| const { searchText, setSearchText } = useFormStore(); | |
| const searchText = useFormStore((state) => state.searchText); | |
| const setSearchText = useFormStore((state) => state.setSearchText); |
| const { startDate, endDate, searchText, setSearchText } = useFormStore(); | ||
|
|
There was a problem hiding this comment.
Using useFormStore() without a selector subscribes SummaryView to the entire store, which can cause expensive re-renders when unrelated state changes (loading, counts, etc.). Prefer selecting only the needed slices (here: startDate, endDate, searchText, setSearchText), using shallow for the multi-field selection.
| setApiMode: (mode: ApiMode) => void; | ||
| setSearchText: (text: string) => void; | ||
| handleSearch: () => void; | ||
| validateUsernameFormat: (username: string) => void; | ||
| addAvatarsToCache: (avatarUrls: { [username: string]: string }) => void; |
There was a problem hiding this comment.
handleSearch is typed as returning void, but the real handleSearch coming from useGitHubDataFetching returns a Promise<void> (and App.tsx uses .then() on it). To keep signatures consistent and allow consumers to await it, update the store action type to return Promise<void>.
| // Sync state and actions from hooks into the zustand store so that | ||
| // child components can subscribe to individual slices without a Provider. | ||
| useEffect(() => { | ||
| useFormStore.setState({ | ||
| username, startDate, endDate, githubToken, apiMode, searchText, | ||
| loading, loadingProgress, error, | ||
| searchItemsCount, eventsCount, rawEventsCount, | ||
| setUsername, setStartDate, setEndDate, setGithubToken, setApiMode, setSearchText, | ||
| handleSearch, validateUsernameFormat, addAvatarsToCache, | ||
| }); |
There was a problem hiding this comment.
Syncing the store from App via useEffect means children render once with the store's default state and no-op action stubs, and only get real values after the effect runs. This can cause a brief UI flicker and (in edge cases) user interactions on the first paint to be dropped. Consider using useLayoutEffect for the sync (or otherwise ensuring the store is populated before interactive children mount).
📝 WalkthroughWalkthroughThis pull request migrates form state management from React Context to Zustand store. A new store file is created, context-based infrastructure (FormContext, useFormContext) is removed from App.tsx, and all consumer components and tests are updated to use the store instead, while adding zustand as a dependency. Changes
Sequence DiagramsequenceDiagram
participant App as App.tsx
participant Store as Zustand Store
participant Component as Components<br/>(SearchForm, etc.)
App->>Store: useEffect: sync form state<br/>& handlers into store via setState()
Component->>Store: Import useFormStore
Component->>Store: Call useFormStore() to<br/>subscribe to store slices
Store-->>Component: Return searchText, setSearchText,<br/>token, dates, handlers, etc.
Component->>Component: Render with store state
Component->>Store: Call setters/handlers<br/>(e.g., setSearchText)
Store->>Store: Update state
Store-->>Component: Notify subscribers<br/>(re-render)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/__tests__/Summary.test.tsx (1)
7-27:⚠️ Potential issue | 🟡 MinorMock store is missing
searchText/setSearchText.
SummaryView destructures these fromuseFormStore, so the mock should include them to avoid undefined behavior in tests.✅ Suggested fix
const mockFormStore = { githubToken: 'test-token', startDate: '2024-01-01', endDate: '2024-01-31', username: 'testuser', + searchText: '', apiMode: 'summary' as const, + setSearchText: vi.fn(), setUsername: vi.fn(), setStartDate: vi.fn(), setEndDate: vi.fn(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/__tests__/Summary.test.tsx` around lines 7 - 27, The mockFormStore used in the Summary.test.tsx is missing the searchText and setSearchText properties which SummaryView destructures from useFormStore; update the mockFormStore object to include searchText (e.g., default string) and setSearchText (a vi.fn()), so SummaryView and any tests that call useFormStore see those keys and avoid undefined behavior when rendering or interacting with components.
🧹 Nitpick comments (3)
src/store/useFormStore.ts (1)
61-70: Avoid silent no-op actions when the store isn’t synced.
Consider a dev-time guard (warn/throw) so mis-wiring doesn’t fail silently ifuseFormStoreSyncisn’t invoked.💡 Example guard to surface missing sync
+const createStub = (name: keyof FormStoreActions) => () => { + if (import.meta.env?.DEV) { + console.warn(`useFormStore action "${name}" called before sync`); + } +}; ... - setUsername: () => {}, - setStartDate: () => {}, - setEndDate: () => {}, - setGithubToken: () => {}, - setApiMode: () => {}, - setSearchText: () => {}, - handleSearch: () => {}, - validateUsernameFormat: () => {}, - addAvatarsToCache: () => {}, + setUsername: createStub('setUsername'), + setStartDate: createStub('setStartDate'), + setEndDate: createStub('setEndDate'), + setGithubToken: createStub('setGithubToken'), + setApiMode: createStub('setApiMode'), + setSearchText: createStub('setSearchText'), + handleSearch: createStub('handleSearch'), + validateUsernameFormat: createStub('validateUsernameFormat'), + addAvatarsToCache: createStub('addAvatarsToCache'),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/store/useFormStore.ts` around lines 61 - 70, Replace the silent no-op action stubs in useFormStore (setUsername, setStartDate, setEndDate, setGithubToken, setApiMode, setSearchText, handleSearch, validateUsernameFormat, addAvatarsToCache) with dev-time guards that surface when the store hasn’t been synced (useFormStoreSync); implement each stub to, when NODE_ENV !== 'production', either console.error/console.warn (including the action name and a hint to call useFormStoreSync) or throw an Error to fail fast, and in production fall back to a no-op to avoid breaking users — update the stub implementations to reference their own names so logs clearly point to the missing sync.src/components/__tests__/SearchForm-RaceCondition.test.tsx (1)
28-48: Minor inconsistency:errorvalue differs from other mocks.This mock sets
error: ''(empty string) at line 43, while other test mocks useerror: null(e.g.,setup.tsline 36,test-utils.tsxline 26). IfFormStore.erroris typed asstring | null, consider usingnullconsistently across all mocks to avoid potential type mismatches or subtle test behavior differences.Suggested fix for consistency
validateUsernameFormat: mockValidateUsernameFormat, addAvatarsToCache: mockAddAvatarsToCache, loading: false, - error: '', + error: null, searchItemsCount: 0, rawEventsCount: 0, githubToken: 'test-token',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/__tests__/SearchForm-RaceCondition.test.tsx` around lines 28 - 48, The mock of the zustand store (useFormStore) sets error: '' but other test mocks use error: null, causing a potential type/behavior mismatch; update the mock in SearchForm-RaceCondition.test.tsx (the vi.mock returning useFormStore) to set error: null to match the FormStore error type and other test setups (e.g., setup.ts/test-utils.tsx) so tests are consistent.src/components/SearchForm.test.tsx (1)
30-48: Mock is missing fields present in other test mocks and the FormStore type.The mock is missing
githubToken,eventsCount, andloadingProgresswhich are present in the store type and in other test file mocks (e.g.,SearchForm-RaceCondition.test.tsxline 46,setup.tslines 22, 35, 38). IfSearchFormaccesses these fields, tests may behave unexpectedly.Suggested additions for consistency
addAvatarsToCache: mockAddAvatarsToCache, loading: false, error: null, searchItemsCount: 0, rawEventsCount: 0, + eventsCount: 0, + loadingProgress: '', + githubToken: 'test-token', }), }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SearchForm.test.tsx` around lines 30 - 48, The test mock for useFormStore is missing fields from the FormStore type which can cause unexpected behavior; update the mock returned by useFormStore in SearchForm.test.tsx to include githubToken, eventsCount, and loadingProgress (and ensure their corresponding setter/mutator mocks exist or are no-ops) so it matches other test mocks (e.g., SearchForm-RaceCondition.test.tsx and setup.ts) and the FormStore interface; locate the useFormStore mock definition and add these properties with sensible defaults consistent with other tests (e.g., githubToken: '', eventsCount: 0, loadingProgress: 0) and corresponding mock setter functions if used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/App.tsx`:
- Around line 111-127: The effect in App.tsx keeps rerunning because
setSearchText (from useLocalStorage) is an unstable setter; memoize that
function to stabilize the dependency: update useLocalStorage to wrap the setter
in useCallback (or expose a memoized setter variant) so the reference returned
as setSearchText is stable, then keep the useEffect that calls
useFormStore.setState unchanged; locate useLocalStorage and the exported
setSearchText name and ensure it returns a useCallback-wrapped setter used by
useGitHubFormState/useEffect.
In `@src/test/setup.ts`:
- Around line 15-40: The current test setup seeds the zustand store once via
useFormStore.setState but never resets it between tests, so state mutations
leak; update the teardown to reset the store after each test by adding a call to
restore the default form state (either call useFormStore.setState with the same
default object used in this file or invoke the shared seedFormStore() helper
from test-utils.tsx) inside the existing afterEach (in addition to
vi.clearAllMocks()), referencing useFormStore and/or seedFormStore so the store
values (username, startDate, endDate, githubToken, apiMode, etc.) are restored
for each test run.
---
Outside diff comments:
In `@src/components/__tests__/Summary.test.tsx`:
- Around line 7-27: The mockFormStore used in the Summary.test.tsx is missing
the searchText and setSearchText properties which SummaryView destructures from
useFormStore; update the mockFormStore object to include searchText (e.g.,
default string) and setSearchText (a vi.fn()), so SummaryView and any tests that
call useFormStore see those keys and avoid undefined behavior when rendering or
interacting with components.
---
Nitpick comments:
In `@src/components/__tests__/SearchForm-RaceCondition.test.tsx`:
- Around line 28-48: The mock of the zustand store (useFormStore) sets error: ''
but other test mocks use error: null, causing a potential type/behavior
mismatch; update the mock in SearchForm-RaceCondition.test.tsx (the vi.mock
returning useFormStore) to set error: null to match the FormStore error type and
other test setups (e.g., setup.ts/test-utils.tsx) so tests are consistent.
In `@src/components/SearchForm.test.tsx`:
- Around line 30-48: The test mock for useFormStore is missing fields from the
FormStore type which can cause unexpected behavior; update the mock returned by
useFormStore in SearchForm.test.tsx to include githubToken, eventsCount, and
loadingProgress (and ensure their corresponding setter/mutator mocks exist or
are no-ops) so it matches other test mocks (e.g.,
SearchForm-RaceCondition.test.tsx and setup.ts) and the FormStore interface;
locate the useFormStore mock definition and add these properties with sensible
defaults consistent with other tests (e.g., githubToken: '', eventsCount: 0,
loadingProgress: 0) and corresponding mock setter functions if used.
In `@src/store/useFormStore.ts`:
- Around line 61-70: Replace the silent no-op action stubs in useFormStore
(setUsername, setStartDate, setEndDate, setGithubToken, setApiMode,
setSearchText, handleSearch, validateUsernameFormat, addAvatarsToCache) with
dev-time guards that surface when the store hasn’t been synced
(useFormStoreSync); implement each stub to, when NODE_ENV !== 'production',
either console.error/console.warn (including the action name and a hint to call
useFormStoreSync) or throw an Error to fail fast, and in production fall back to
a no-op to avoid breaking users — update the stub implementations to reference
their own names so logs clearly point to the missing sync.
| // Sync state and actions from hooks into the zustand store so that | ||
| // child components can subscribe to individual slices without a Provider. | ||
| useEffect(() => { | ||
| useFormStore.setState({ | ||
| username, startDate, endDate, githubToken, apiMode, searchText, | ||
| loading, loadingProgress, error, | ||
| searchItemsCount, eventsCount, rawEventsCount, | ||
| setUsername, setStartDate, setEndDate, setGithubToken, setApiMode, setSearchText, | ||
| handleSearch, validateUsernameFormat, addAvatarsToCache, | ||
| }); | ||
| }, [ | ||
| username, startDate, endDate, githubToken, apiMode, searchText, | ||
| loading, loadingProgress, error, | ||
| searchItemsCount, eventsCount, rawEventsCount, | ||
| setUsername, setStartDate, setEndDate, setGithubToken, setApiMode, setSearchText, | ||
| handleSearch, validateUsernameFormat, addAvatarsToCache, | ||
| ]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f -e ts -e tsx -e js -e jsx | xargs rg "useGitHubFormState|const useGitHubFormState" -lRepository: kertal/git-vegas
Length of output: 158
🏁 Script executed:
cat -n src/hooks/useGitHubFormState.tsRepository: kertal/git-vegas
Length of output: 9580
🏁 Script executed:
cat -n src/App.tsx | head -150 | tail -80Repository: kertal/git-vegas
Length of output: 3041
🏁 Script executed:
cat -n src/App.tsx | head -70Repository: kertal/git-vegas
Length of output: 3090
🏁 Script executed:
cat -n src/hooks/useLocalStorage.tsRepository: kertal/git-vegas
Length of output: 10236
setSearchText from useLocalStorage is not memoized, causing unnecessary effect reruns.
Most setter functions from useGitHubFormState are properly wrapped in useCallback. However, setSearchText comes from useLocalStorage (line 42) and is the raw React setState function—not memoized. Since this unstable reference is included in the effect dependency array (line 125), the effect runs on every render, unnecessarily syncing to Zustand even when no actual state changed.
Wrap setSearchText with useCallback in useLocalStorage or consider extracting the setter memoization logic into a custom hook to ensure stable references.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/App.tsx` around lines 111 - 127, The effect in App.tsx keeps rerunning
because setSearchText (from useLocalStorage) is an unstable setter; memoize that
function to stabilize the dependency: update useLocalStorage to wrap the setter
in useCallback (or expose a memoized setter variant) so the reference returned
as setSearchText is stable, then keep the useEffect that calls
useFormStore.setState unchanged; locate useLocalStorage and the exported
setSearchText name and ensure it returns a useCallback-wrapped setter used by
useGitHubFormState/useEffect.
| // Seed the zustand form store with default test values | ||
| import { useFormStore } from '../store/useFormStore'; | ||
|
|
||
| useFormStore.setState({ | ||
| username: 'testuser', | ||
| startDate: '2023-01-01', | ||
| endDate: '2023-12-31', | ||
| githubToken: 'test-token', | ||
| apiMode: 'search', | ||
| searchText: '', | ||
| setUsername: vi.fn(), | ||
| setStartDate: vi.fn(), | ||
| setEndDate: vi.fn(), | ||
| setGithubToken: vi.fn(), | ||
| setApiMode: vi.fn(), | ||
| setSearchText: vi.fn(), | ||
| handleSearch: vi.fn(), | ||
| validateUsernameFormat: vi.fn(), | ||
| addAvatarsToCache: vi.fn(), | ||
| loading: false, | ||
| loadingProgress: '', | ||
| error: null, | ||
| searchItemsCount: 0, | ||
| eventsCount: 0, | ||
| rawEventsCount: 0, | ||
| }); |
There was a problem hiding this comment.
Store state is not reset between tests, which may cause test isolation issues.
The store is seeded once at module load (lines 18-40), but the afterEach hook (line 412) only calls vi.clearAllMocks() which resets the mock functions, not the store's state values (username, startDate, etc.). If any test modifies the store state directly, those changes will leak into subsequent tests.
Consider resetting the store in afterEach to ensure test isolation:
Suggested fix
afterEach(() => {
// Clear all mocks
vi.clearAllMocks();
// Cleanup React Testing Library
cleanup();
// Clear storages
localStorageMock.clear();
sessionStorageMock.clear();
// Reset location
locationMock.href = BASE_URL;
+ // Reset zustand store to default test values
+ useFormStore.setState({
+ username: 'testuser',
+ startDate: '2023-01-01',
+ endDate: '2023-12-31',
+ githubToken: 'test-token',
+ apiMode: 'search',
+ searchText: '',
+ setUsername: vi.fn(),
+ setStartDate: vi.fn(),
+ setEndDate: vi.fn(),
+ setGithubToken: vi.fn(),
+ setApiMode: vi.fn(),
+ setSearchText: vi.fn(),
+ handleSearch: vi.fn(),
+ validateUsernameFormat: vi.fn(),
+ addAvatarsToCache: vi.fn(),
+ loading: false,
+ loadingProgress: '',
+ error: null,
+ searchItemsCount: 0,
+ eventsCount: 0,
+ rawEventsCount: 0,
+ });
});Alternatively, import and use seedFormStore() from test-utils.tsx to avoid duplication.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/test/setup.ts` around lines 15 - 40, The current test setup seeds the
zustand store once via useFormStore.setState but never resets it between tests,
so state mutations leak; update the teardown to reset the store after each test
by adding a call to restore the default form state (either call
useFormStore.setState with the same default object used in this file or invoke
the shared seedFormStore() helper from test-utils.tsx) inside the existing
afterEach (in addition to vi.clearAllMocks()), referencing useFormStore and/or
seedFormStore so the store values (username, startDate, endDate, githubToken,
apiMode, etc.) are restored for each test run.
Summary
Migrated form state management from React Context (
FormContext) to Zustand store (useFormStore), eliminating the need for a context provider while maintaining the same functionality and improving component subscription patterns.Key Changes
FormContextanduseFormContexthook fromApp.tsx, along with the context provider wrapper around child componentssrc/store/useFormStore.tswith a centralized store containing all form state and action callbacksuseEffecthook that syncs state from existing hooks into the Zustand store, keeping hooks as the source of truth while enabling fine-grained subscriptions in child componentsuseFormContext()imports withuseFormStore()in:SearchForm.tsxSettingsDialog.tsxEventView.tsxIssuesAndPRsList.tsxSummary.tsxtest/setup.tsand replaced with direct Zustand store seedingtest-utils.tsxto remove context providers and addedseedFormStore()helperuseFormStoreinstead ofuseFormContextFormContextTypeandResultsContextTypefromtypes.ts(context types now live in store file)zustand@^5.0.11topackage.jsonImplementation Details
The migration maintains backward compatibility by:
useEffect, so the hooks remain the source of truthhttps://claude.ai/code/session_01BM8QfufDNGMCvJPVBEAw5w
Summary by CodeRabbit
Release Notes