[use-effect] fix(web): replace reset Effect with render-time state adjustment in RepositoryPicker - #144
Open
github-actions[bot] wants to merge 2 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MaggieAppleton
marked this pull request as ready for review
September 3, 2026 07:27
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Effect location and classification
RepositoryPickerinapps/web/src/repository-picker.tsx(around line 100)had:
This is a reset-or-adjust-state Effect: it resets local
avatarFailedstate whenever the
current?.ownerAvatarUrlprop changes, so a previousonErrorfailure for one repository's avatar doesn't leak into the nextrepository shown.
Why it was incorrect
This is a textbook "adjusting state when a prop changes" case that the React
docs explicitly call out as not needing an Effect. Using
useEffectheremeans React renders once with stale
avatarFailedstate, then re-rendersafter the Effect runs — an extra render pass and a flash of the fallback
avatar/initial before the reset takes effect.
Selected refactor
Replaced the Effect with the recommended "adjust state during render" pattern:
track the previous
ownerAvatarUrlin a piece of state, and if it differsfrom the current prop, call
setAvatarFailed(and update the tracked value)directly during render. React discards this render and immediately re-renders
with the updated state, avoiding the extra Effect-driven render/flash while
keeping the reset logic co-located with the state it adjusts.
Verification
Ran a scoped
tsc --noEmit --skipLibChecktypecheck overapps/web(
bun/tsgowere unavailable in this sandbox, so the workspace's pinnedTypeScript binary was invoked directly as the closest equivalent to
bun run types): no errors were reported, including none forrepository-picker.tsx.Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
releaseassets.githubusercontent.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.