Skip to content

fix(frontend): resolve set-state-in-effect in ScanResultView (#1063)#1073

Open
ericsocrat wants to merge 1 commit intomainfrom
chore/react-compiler-scan-result-view
Open

fix(frontend): resolve set-state-in-effect in ScanResultView (#1063)#1073
ericsocrat wants to merge 1 commit intomainfrom
chore/react-compiler-scan-result-view

Conversation

@ericsocrat
Copy link
Copy Markdown
Owner

Phase 2 of #1063 — fifth of 19 react-hooks/set-state-in-effect violations. Resolves two violations in a single file (lines 34 and 216).

Change

Wraps two synchronous effect setState calls in requestAnimationFrame so they are asynchronous and no longer violate the rule:

  1. FadeSlideIn (line 34): mount-time setVisible(true) to trigger the fade-in transition.
  2. Animated score count-up (line 216): early-return branch when prefersReduced or tryVitScore === 0.

Both effects now also cancelAnimationFrame on unmount. Visual outcome is unchanged — the fade-in still runs from opacity 0 on the next frame, and the no-animation case still snaps the score to its target value.

Verification

  • eslint src/components/scan/ScanResultView.tsx — clean
  • vitest run src/components/scan/ScanResultView.test.tsx — 33/33 pass
  • tsc --noEmit — clean

Progress

Wraps two synchronous effect setState calls in requestAnimationFrame so they become asynchronous and no longer trip react-hooks/set-state-in-effect: the mount-time fade-in in FadeSlideIn, and the no-animation early return in the animated score count-up. Visual behaviour is unchanged. Both effects now also clean up the rAF on unmount.
Copilot AI review requested due to automatic review settings May 1, 2026 10:48
@ericsocrat ericsocrat enabled auto-merge (squash) May 1, 2026 10:48
@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 10:49am

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 1, 2026

📸 PR Screenshots

Captured 2 screenshots (1 mobile, 1 desktop) for 1 changed page(s):

  • scanner

📥 Download screenshots from workflow artifacts.

Captured by PR Screenshots workflow • 2026-05-01

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 addresses two remaining react-hooks/set-state-in-effect warnings in ScanResultView.tsx as part of the ongoing React Compiler lint-drain effort (#1063), while preserving the existing UI behavior (fade-in transition + score snap/count-up).

Changes:

  • Updated FadeSlideIn to trigger its mount-time visibility flip via requestAnimationFrame (and cancel on unmount).
  • Updated the “no animation” branch of the score count-up effect to set the final score via requestAnimationFrame (and cancel on unmount).

@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