Repository navigation
Fix webview comment and stack action lifecycle - #9049
Open
Henning Dieterichs (hediet) wants to merge 3 commits into
Open
Henning Dieterichs (hediet) wants to merge 3 commits into
Henning Dieterichs (hediet) wants to merge 3 commits into
Conversation
Clean up quote-reply subscriptions, preserve hook order, wire controlled drafts through onChange, assign stable avatar keys, and avoid stack-action state updates after unmount. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot started reviewing on behalf of
Henning Dieterichs (hediet)
October 9, 2026 17:01
View session
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Oct 9, 2026
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The lifecycle fixes are appropriately scoped and covered by targeted regression tests.
0 open findings
What changed in this PR
Fixes repeated-render lifecycle issues in webview comments, sidebar avatars, and stack actions.
Changes:
- Stabilizes hook, subscription, and controlled-textarea behavior.
- Adds stable avatar keys and guards asynchronous stack state updates after unmount.
- Adds focused regression tests for these lifecycle scenarios.
| File | Description |
|---|---|
webviews/components/comment.tsx |
Fixes hooks, event cleanup, and textarea changes. |
webviews/components/sidebar.tsx |
Adds stable avatar keys. |
webviews/components/pullRequestStack.tsx |
Prevents state updates after unmount. |
webviews/components/test/comment.test.tsx |
Tests drafts and subscription cleanup. |
webviews/components/test/sidebar.test.tsx |
Tests duplicate avatar-key prevention. |
webviews/components/test/pullRequestStack.test.tsx |
Tests pending-action disposal. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Copilot started reviewing on behalf of
Henning Dieterichs (hediet)
October 9, 2026 17:07
View session
| const sections: { label: string; value: React.ReactNode; count: number }[] = []; | ||
|
|
||
| const reviewersWithAvatar = reviewers?.filter((r): r is ReviewState & { reviewer: { avatarUrl: string } } => !!r.reviewer.avatarUrl).map(r => ({ avatarUrl: r.reviewer.avatarUrl, name: reviewerLabel(r.reviewer) })); | ||
| const reviewersWithAvatar = reviewers?.filter((r): r is ReviewState & { reviewer: { avatarUrl: string } } => !!r.reviewer.avatarUrl).map(r => ({ id: reviewerId(r.reviewer), avatarUrl: r.reviewer.avatarUrl, name: reviewerLabel(r.reviewer) })); |
Update the preparation dependency without pulling Explorer-only responsive changes into this PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot started reviewing on behalf of
Henning Dieterichs (hediet)
October 9, 2026 18:16
View session
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.
Summary
Fix product issues exposed by rendering webviews repeatedly in Component Explorer, without adding Explorer tooling to this PR.
Relationship to the stack
This is separate from #9047 (preparation) and #9048 (Explorer adoption). It targets the preparation branch because its tests use the explicit host helper. The changes were deliberately excluded from those PRs and are owned here only. Merge preparation first, then this fix, then Explorer; retarget to main as the prerequisite lands.
Validation
npm run test:webviews: build succeeded and all 86 tests passed using locked dependencies from the preparation worktree.npm run lintandnpm run hygienepassed.The preparation dependency now builds on #9050, which independently owns the pending-reply Map/leak fix. That change is not part of this PR diff.
The viewport-only preparation was moved into #9048. This branch has been updated to the narrower preparation dependency; its product-fix additions and deletions are unchanged.