Skip to content

fix(frontend): resolve set-state-in-effect in CommandPalette (#1063)#1076

Open
ericsocrat wants to merge 1 commit intomainfrom
chore/react-compiler-command-palette
Open

fix(frontend): resolve set-state-in-effect in CommandPalette (#1063)#1076
ericsocrat wants to merge 1 commit intomainfrom
chore/react-compiler-command-palette

Conversation

@ericsocrat
Copy link
Copy Markdown
Owner

Phase 2 of #1063 — eighth PR, resolves 9 + 10 of 19 violations.

Two react-hooks/set-state-in-effect warnings in CommandPalette.tsx resolved by adjusting state during render instead of inside useEffect:

  1. Reset query/activeIndex on open transition — replaced the mixed setState+DOM effect with a prevOpen render-phase tracker. The imperative <dialog> showModal/close/focus stays in a separate effect (DOM-only, no setState).
  2. Keep activeIndex in boundsprevFilteredLen render-phase tracker; clamp during render when filteredItems.length shrinks.

Pattern reference: https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes

Behaviour unchanged. 16/16 vitest pass, eslint and tsc clean.

Cross-references prior Phase 2 PRs: #1067, #1069, #1070, #1071, #1072, #1073, #1074, #1075.

Two violations resolved by adjusting state during render instead of inside useEffect:

1. Reset query/activeIndex on open transition - prevOpen tracker; the imperative <dialog> showModal/close/focus stays in a separate effect (DOM-only, no setState).

2. Keep activeIndex in bounds on filteredItems.length change - prevFilteredLen tracker, clamp during render.

Behaviour unchanged. 16/16 tests pass, eslint and tsc clean.
Copilot AI review requested due to automatic review settings May 1, 2026 11:20
@vercel
Copy link
Copy Markdown

vercel Bot commented May 1, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
tryvit Ready Ready Preview, Comment May 1, 2026 11:22am

Copy link
Copy Markdown

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

This PR updates the CommandPalette component to eliminate react-hooks/set-state-in-effect warnings by moving two pieces of state synchronization (reset-on-open and activeIndex clamping) from useEffect into render-phase state adjustments, while keeping imperative <dialog> DOM operations in an effect.

Changes:

  • Reset query and activeIndex on the closed→open transition via a prevOpen render-phase tracker.
  • Clamp activeIndex when filteredItems.length shrinks via a prevFilteredLen render-phase tracker.
  • Keep <dialog> showModal/close and focus behavior in a DOM-only useEffect.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 1, 2026

Bundle Size Report

Metric Value
Main baseline 0 KB
This PR 0 KB
Delta +0 KB (+0%)
JS chunks 0
Hard limit 4000 KB

✅ Bundle size is within acceptable limits.

ericsocrat added a commit that referenced this pull request May 1, 2026
…) (#1077)

Phase 2 of #1063 -- resolves 2 of 19 violations (running total: 12/19 across 9 PRs).

Two react-hooks/set-state-in-effect warnings in SearchAutocomplete.tsx resolved by adjusting state during render:

1. Refresh recent-searches list on dropdown open: replaced the show-dep useEffect with a prevShow render-phase tracker. Added a lazy initial-state initializer so the list is hydrated on mount when show is already true (preserves existing test contract).

2. Reset activeIndex when suggestions/query change: replaced the multi-dep useEffect with a composite resetKey + prevResetKey render-phase tracker.

Pattern: https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes

Behaviour unchanged. 32/32 vitest pass, eslint and tsc clean.

Cross-references: #1067, #1069, #1070, #1071, #1072, #1073, #1074, #1075, #1076.

Co-authored-by: ericsocrat <ericsocrat@users.noreply.github.com>
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.

2 participants