diff --git a/webviews/components/comment.tsx b/webviews/components/comment.tsx
index 6390c4b571..1d4c9bf3fd 100644
--- a/webviews/components/comment.tsx
+++ b/webviews/components/comment.tsx
@@ -425,6 +425,7 @@ export interface Embodied {
}
export const CommentBody = ({ comment, bodyHTML, body, canApplyPatch, allowEmpty, specialDisplayBodyPostfix }: Embodied) => {
+ const { applyPatch } = useContext(PullRequestContext);
if (!body && !bodyHTML) {
if (allowEmpty) {
return null;
@@ -436,7 +437,6 @@ export const CommentBody = ({ comment, bodyHTML, body, canApplyPatch, allowEmpty
);
}
- const { applyPatch } = useContext(PullRequestContext);
const renderedBody =
;
const containsSuggestion = ((body || bodyHTML)?.indexOf('```diff') ?? -1) > -1;
@@ -514,12 +514,18 @@ export function AddComment({
const form = useRef();
const textareaRef = useRef();
- emitter.addListener('quoteReply', (message: string) => {
- const quoted = message.replace(/\n/g, '\n> ');
- updatePR({ pendingCommentText: `> ${quoted} \n\n` });
- textareaRef.current?.scrollIntoView();
- textareaRef.current?.focus();
- });
+ useEffect(() => {
+ const quoteReply = (message: string) => {
+ const quoted = message.replace(/\n/g, '\n> ');
+ updatePR({ pendingCommentText: `> ${quoted} \n\n` });
+ textareaRef.current?.scrollIntoView();
+ textareaRef.current?.focus();
+ };
+ emitter.addListener('quoteReply', quoteReply);
+ return () => {
+ emitter.removeListener('quoteReply', quoteReply);
+ };
+ }, [updatePR]);
const closeButton: React.MouseEventHandler = e => {
e.preventDefault();
@@ -583,7 +589,7 @@ export function AddComment({
id={COMMENT_TEXTAREA_ID}
name="body"
ref={textareaRef as React.MutableRefObject}
- onInput={({ target }) => updatePR({ pendingCommentText: (target as HTMLTextAreaElement).value })}
+ onChange={event => updatePR({ pendingCommentText: event.currentTarget.value })}
onKeyDown={onKeyDown}
onPaste={onPasteUploadFiles(uploadPastedFilesIntoPendingComment)}
value={pendingCommentText}
diff --git a/webviews/components/pullRequestStack.tsx b/webviews/components/pullRequestStack.tsx
index 84c77501ff..34570b3eba 100644
--- a/webviews/components/pullRequestStack.tsx
+++ b/webviews/components/pullRequestStack.tsx
@@ -50,6 +50,11 @@ export const StackSection = ({ pr }: { pr: PullRequest }) => {
const [error, setError] = React.useState();
const [updating, setUpdating] = React.useState(false);
const [updateError, setUpdateError] = React.useState();
+ const mounted = React.useRef(true);
+ React.useLayoutEffect(() => {
+ mounted.current = true;
+ return () => { mounted.current = false; };
+ }, []);
const { stack } = pr;
if (!stack) {
return null;
@@ -65,9 +70,13 @@ export const StackSection = ({ pr }: { pr: PullRequest }) => {
setUpdateError(undefined);
await unstackAll();
} catch (unstackError) {
- setError(`Unable to unstack pull requests: ${unstackError instanceof Error ? unstackError.message || unstackError.name : String(unstackError)}`);
+ if (mounted.current) {
+ setError(`Unable to unstack pull requests: ${unstackError instanceof Error ? unstackError.message || unstackError.name : String(unstackError)}`);
+ }
} finally {
- setBusy(false);
+ if (mounted.current) {
+ setBusy(false);
+ }
}
};
@@ -78,9 +87,13 @@ export const StackSection = ({ pr }: { pr: PullRequest }) => {
setError(undefined);
await updateStack();
} catch (updateFailure) {
- setUpdateError(`Unable to update the stack: ${updateFailure instanceof Error ? updateFailure.message || updateFailure.name : String(updateFailure)}`);
+ if (mounted.current) {
+ setUpdateError(`Unable to update the stack: ${updateFailure instanceof Error ? updateFailure.message || updateFailure.name : String(updateFailure)}`);
+ }
} finally {
- setUpdating(false);
+ if (mounted.current) {
+ setUpdating(false);
+ }
}
};
diff --git a/webviews/components/sidebar.tsx b/webviews/components/sidebar.tsx
index e776a2e4d0..ef773445f1 100644
--- a/webviews/components/sidebar.tsx
+++ b/webviews/components/sidebar.tsx
@@ -346,12 +346,12 @@ function CollapsedLabel(props: PullRequest) {
return () => window.removeEventListener('resize', checkViewportWidth);
}, []);
- const AvatarStack = ({ users }: { users: { avatarUrl: string; name: string }[] }) => (
+ const AvatarStack = ({ users }: { users: { id: string; avatarUrl: string; name: string }[] }) => (
{users.slice(0, 10).map((u, i) => (
-
@@ -430,7 +430,7 @@ function CollapsedLabel(props: PullRequest) {
// Collect non-empty sections in order, with custom rendering
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) }));
if (!isIssue && reviewersWithAvatar && reviewersWithAvatar.length) {
sections.push({
label: 'Reviewers',
@@ -439,7 +439,7 @@ function CollapsedLabel(props: PullRequest) {
});
}
- const assigneesWithAvatar = assignees?.filter((a): a is IAccount & { avatarUrl: string; login: string } => !!a.avatarUrl).map(a => ({ avatarUrl: a.avatarUrl, name: reviewerLabel(a) }));
+ const assigneesWithAvatar = assignees?.filter((a): a is IAccount & { avatarUrl: string; login: string } => !!a.avatarUrl).map(a => ({ id: a.id, avatarUrl: a.avatarUrl, name: reviewerLabel(a) }));
if (assigneesWithAvatar && assigneesWithAvatar.length) {
sections.push({
label: 'Assignees',
diff --git a/webviews/components/test/comment.test.tsx b/webviews/components/test/comment.test.tsx
new file mode 100644
index 0000000000..0b410bfe7d
--- /dev/null
+++ b/webviews/components/test/comment.test.tsx
@@ -0,0 +1,75 @@
+/*---------------------------------------------------------------------------------------------
+ * Copyright (c) Microsoft Corporation. All rights reserved.
+ * Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import assert from 'assert';
+import * as React from 'react';
+import { act, cleanup, fireEvent, render, wait } from 'react-testing-library';
+import { createSandbox } from 'sinon';
+import { createTestHost } from '../../../src/test/webviews/testHost';
+import PullRequestContext, { PRContext } from '../../common/context';
+import emitter from '../../common/events';
+import { vscodeTransport as vscode } from '../../common/host';
+import { PullRequestBuilder } from '../../editorWebview/test/builder/pullRequest';
+import { AddComment } from '../comment';
+
+describe('Comment composer quote replies', () => {
+ afterEach(() => {
+ cleanup();
+ vscode.setState(undefined);
+ });
+
+ it('updates a controlled draft through the React change event without warnings', () => {
+ const sandbox = createSandbox();
+ const context = new PRContext(createTestHost(new PullRequestBuilder().build()));
+ context.updatePR({ pendingCommentText: 'Existing draft' });
+ const errors = sandbox.stub(console, 'error');
+ const out = render(
+
+ );
+ try {
+ const textarea = out.container.querySelector('textarea')!;
+ assert.strictEqual(textarea.value, 'Existing draft');
+ fireEvent.change(textarea, { target: { value: 'Updated draft' } });
+ assert.strictEqual(context.pr!.pendingCommentText, 'Updated draft');
+ assert.deepStrictEqual(errors.args, []);
+ } finally {
+ out.unmount();
+ context.dispose();
+ sandbox.restore();
+ }
+ });
+
+ it('registers once across rerenders and removes its listener on unmount', async () => {
+ const sandbox = createSandbox();
+ const pr = new PullRequestBuilder().build();
+ const context = new PRContext(createTestHost(pr));
+ const update = sandbox.spy(context, 'updatePR');
+ const previousListeners = emitter.listenerCount('quoteReply');
+ const view = (busy: boolean) =>
+
+ ;
+ const out = render(view(false));
+ try {
+ await wait(() => assert.strictEqual(emitter.listenerCount('quoteReply'), previousListeners + 1));
+ out.rerender(view(true));
+ out.rerender(view(false));
+ assert.strictEqual(emitter.listenerCount('quoteReply'), previousListeners + 1);
+ const textarea = out.container.querySelector('textarea')!;
+ textarea.scrollIntoView = sandbox.spy();
+ act(() => { emitter.emit('quoteReply', 'First line\nSecond line'); });
+ assert.strictEqual(update.callCount, 1);
+ assert.strictEqual(context.pr!.pendingCommentText, '> First line\n> Second line \n\n');
+ assert.strictEqual(document.activeElement, textarea);
+ out.unmount();
+ await wait(() => assert.strictEqual(emitter.listenerCount('quoteReply'), previousListeners));
+ emitter.emit('quoteReply', 'After unmount');
+ assert.strictEqual(update.callCount, 1);
+ } finally {
+ out.unmount();
+ context.dispose();
+ sandbox.restore();
+ }
+ });
+});
diff --git a/webviews/components/test/pullRequestStack.test.tsx b/webviews/components/test/pullRequestStack.test.tsx
new file mode 100644
index 0000000000..974c458074
--- /dev/null
+++ b/webviews/components/test/pullRequestStack.test.tsx
@@ -0,0 +1,54 @@
+/*---------------------------------------------------------------------------------------------
+ * Copyright (c) Microsoft Corporation. All rights reserved.
+ * Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import assert from 'assert';
+import * as React from 'react';
+import { cleanup, fireEvent, render } from 'react-testing-library';
+import { createSandbox } from 'sinon';
+import { GithubItemStateEnum, PullRequestMergeability } from '../../../src/github/interface';
+import { createTestHost } from '../../../src/test/webviews/testHost';
+import PullRequestContext, { PRContext } from '../../common/context';
+import { PullRequestBuilder } from '../../editorWebview/test/builder/pullRequest';
+import { StackSection } from '../pullRequestStack';
+
+describe('Stack action disposal', () => {
+ afterEach(cleanup);
+
+ for (const action of ['Update stack', 'Unstack all']) {
+ it(`does not update an unmounted component when ${action} rejects`, async () => {
+ const sandbox = createSandbox();
+ const pr = new PullRequestBuilder().stack({
+ position: 1, size: 1, base: 'main',
+ pullRequests: [{
+ position: 1, number: 1234, title: 'First', head: 'D1', url: 'https://example.com/1234',
+ state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Mergeable,
+ }],
+ }).build();
+ pr.hasWritePermission = true;
+ pr.canUpdateStack = true;
+ const host = createTestHost(pr);
+ const context = new PRContext(host);
+ let rejectPending!: (error: Error) => void;
+ const pending = new Promise((_, reject) => { rejectPending = reject; });
+ const request = sandbox.stub(host, 'postMessage').returns(pending);
+ const errors = sandbox.stub(console, 'error');
+ const out = render(
+
+ );
+ try {
+ fireEvent.click(out.getByText(action));
+ assert.strictEqual(request.callCount, 1);
+ out.unmount();
+ rejectPending(new Error('Host disposed'));
+ await new Promise(resolve => setTimeout(resolve, 0));
+ assert.deepStrictEqual(errors.args, []);
+ } finally {
+ out.unmount();
+ context.dispose();
+ sandbox.restore();
+ }
+ });
+ }
+});
diff --git a/webviews/components/test/sidebar.test.tsx b/webviews/components/test/sidebar.test.tsx
new file mode 100644
index 0000000000..94a9f39e7c
--- /dev/null
+++ b/webviews/components/test/sidebar.test.tsx
@@ -0,0 +1,36 @@
+/*---------------------------------------------------------------------------------------------
+ * Copyright (c) Microsoft Corporation. All rights reserved.
+ * Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import assert from 'assert';
+import * as React from 'react';
+import { cleanup, render } from 'react-testing-library';
+import { createSandbox } from 'sinon';
+import { createTestHost } from '../../../src/test/webviews/testHost';
+import PullRequestContext, { PRContext } from '../../common/context';
+import { AccountBuilder } from '../../editorWebview/test/builder/account';
+import { PullRequestBuilder } from '../../editorWebview/test/builder/pullRequest';
+import { CollapsibleSidebar } from '../sidebar';
+
+describe('Collapsed sidebar avatars', () => {
+ afterEach(cleanup);
+
+ it('renders accounts with shared names and avatars without duplicate or missing keys', () => {
+ const sandbox = createSandbox();
+ const error = sandbox.spy(console, 'error');
+ const assignees = ['first', 'second'].map(id => new AccountBuilder().id(id).build());
+ const pr = new PullRequestBuilder().assignees(assignees).build();
+ const context = new PRContext(createTestHost(pr));
+ try {
+ const out = render(
+
+ );
+ assert.strictEqual(out.container.querySelectorAll('.stacked-avatar').length, 2);
+ assert(!error.args.some(args => String(args[0]).includes('key')));
+ } finally {
+ context.dispose();
+ sandbox.restore();
+ }
+ });
+});