Skip to content

Commit 5155eb8

Browse files
authored
Fix activity bar webviews from disposable breakage (#6455)
1 parent bbca381 commit 5155eb8

3 files changed

Lines changed: 71 additions & 44 deletions

File tree

src/common/lifecycle.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ export function disposeAll(disposables: vscode.Disposable[]) {
2020
}
2121
}
2222

23+
export function addDisposable<T extends vscode.Disposable>(a: T, disposables: vscode.Disposable[]): T {
24+
disposables.push(a);
25+
return a;
26+
}
27+
2328
export abstract class Disposable {
2429
protected _isDisposed = false;
2530

src/view/createPullRequestHelper.ts

Lines changed: 52 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import * as vscode from 'vscode';
77
import { Repository } from '../api/api';
88
import { commands } from '../common/executeCommands';
9-
import { disposeAll } from '../common/lifecycle';
9+
import { addDisposable, Disposable, disposeAll } from '../common/lifecycle';
1010
import { ITelemetry } from '../common/telemetry';
1111
import { BaseCreatePullRequestViewProvider, BasePullRequestDataModel, CreatePullRequestViewProvider } from '../github/createPRViewProvider';
1212
import { FolderRepositoryManager, PullRequestDefaults } from '../github/folderRepositoryManager';
@@ -15,14 +15,16 @@ import { RevertPullRequestViewProvider } from '../github/revertPRViewProvider';
1515
import { CompareChanges } from './compareChangesTreeDataProvider';
1616
import { CreatePullRequestDataModel } from './createPullRequestDataModel';
1717

18-
export class CreatePullRequestHelper implements vscode.Disposable {
19-
private _disposables: vscode.Disposable[] = [];
18+
export class CreatePullRequestHelper extends Disposable {
19+
private _currentDisposables: vscode.Disposable[] = [];
2020
private _createPRViewProvider: BaseCreatePullRequestViewProvider | undefined;
2121
private _treeView: CompareChanges | undefined;
2222
private _postCreateCallback: ((pullRequestModel: PullRequestModel | undefined) => Promise<void>) | undefined;
2323
private _activeContext: string | undefined;
2424

25-
constructor() { }
25+
constructor() {
26+
super();
27+
}
2628

2729
private async setActiveContext(value: boolean) {
2830
if (this._activeContext) {
@@ -31,97 +33,111 @@ export class CreatePullRequestHelper implements vscode.Disposable {
3133
}
3234

3335
private registerListeners(repository: Repository, usingCurrentBranchAsCompare: boolean) {
34-
this._disposables.push(
36+
addDisposable(
3537
this._createPRViewProvider!.onDone(async createdPR => {
3638
await CreatePullRequestViewProvider.withProgress(async () => {
3739
return this._postCreateCallback?.(createdPR);
3840
});
3941
this.dispose();
4042
}),
43+
this._currentDisposables
4144
);
4245

43-
this._disposables.push(
46+
addDisposable(
4447
vscode.commands.registerCommand('pr.addAssigneesToNewPr', _ => {
4548
return this._createPRViewProvider?.addAssignees();
4649

4750
}),
51+
this._currentDisposables
4852
);
4953

50-
this._disposables.push(
54+
addDisposable(
5155
vscode.commands.registerCommand('pr.addReviewersToNewPr', _ => {
5256
return this._createPRViewProvider?.addReviewers();
5357
}),
58+
this._currentDisposables
5459
);
5560

56-
this._disposables.push(
61+
addDisposable(
5762
vscode.commands.registerCommand('pr.addLabelsToNewPr', _ => {
5863
return this._createPRViewProvider?.addLabels();
5964
}),
65+
this._currentDisposables
6066
);
6167

62-
this._disposables.push(
68+
addDisposable(
6369
vscode.commands.registerCommand('pr.addMilestoneToNewPr', _ => {
6470
return this._createPRViewProvider?.addMilestone();
6571

6672
}),
73+
this._currentDisposables
6774
);
6875

69-
this._disposables.push(
76+
addDisposable(
7077
vscode.commands.registerCommand('pr.addProjectsToNewPr', _ => {
7178
return this._createPRViewProvider?.addProjects();
7279

7380
}),
81+
this._currentDisposables
7482
);
7583

76-
this._disposables.push(
84+
addDisposable(
7785
vscode.commands.registerCommand('pr.createPrMenuCreate', () => {
7886
this._createPRViewProvider?.createFromCommand(false, false, undefined);
7987

80-
})
88+
}),
89+
this._currentDisposables
8190
);
82-
this._disposables.push(
91+
addDisposable(
8392
vscode.commands.registerCommand('pr.createPrMenuDraft', () => {
8493
this._createPRViewProvider?.createFromCommand(true, false, undefined);
8594

86-
})
95+
}),
96+
this._currentDisposables
8797
);
88-
this._disposables.push(
98+
addDisposable(
8999
vscode.commands.registerCommand('pr.createPrMenuMergeWhenReady', () => {
90100
this._createPRViewProvider?.createFromCommand(false, true, undefined, true);
91101

92-
})
102+
}),
103+
this._currentDisposables
93104
);
94-
this._disposables.push(
105+
addDisposable(
95106
vscode.commands.registerCommand('pr.createPrMenuMerge', () => {
96107
this._createPRViewProvider?.createFromCommand(false, true, 'merge');
97108

98-
})
109+
}),
110+
this._currentDisposables
99111
);
100-
this._disposables.push(
112+
addDisposable(
101113
vscode.commands.registerCommand('pr.createPrMenuSquash', () => {
102114
this._createPRViewProvider?.createFromCommand(false, true, 'squash');
103-
})
115+
}),
116+
this._currentDisposables
104117
);
105-
this._disposables.push(
118+
addDisposable(
106119
vscode.commands.registerCommand('pr.createPrMenuRebase', () => {
107120
this._createPRViewProvider?.createFromCommand(false, true, 'rebase');
108-
})
121+
}),
122+
this._currentDisposables
109123
);
110-
this._disposables.push(
124+
addDisposable(
111125
vscode.commands.registerCommand('pr.preReview', () => {
112126
if (this._createPRViewProvider instanceof CreatePullRequestViewProvider) {
113127
this._createPRViewProvider.review();
114128
}
115-
})
129+
}),
130+
this._currentDisposables
116131
);
117132

118133
if (usingCurrentBranchAsCompare) {
119-
this._disposables.push(
134+
addDisposable(
120135
repository.state.onDidChange(_ => {
121136
if (this._createPRViewProvider && repository.state.HEAD && this._createPRViewProvider instanceof CreatePullRequestViewProvider) {
122137
this._createPRViewProvider.setDefaultCompareBranch(repository.state.HEAD);
123138
}
124139
}),
140+
this._currentDisposables
125141
);
126142
}
127143
}
@@ -172,22 +188,23 @@ export class CreatePullRequestHelper implements vscode.Disposable {
172188
baseOwner: pullRequestModel.remote.owner,
173189
repositoryName: pullRequestModel.remote.repositoryName
174190
};
175-
this._createPRViewProvider = new RevertPullRequestViewProvider(
191+
this._createPRViewProvider = addDisposable(new RevertPullRequestViewProvider(
176192
telemetry,
177193
model,
178194
extensionUri,
179195
folderRepoManager,
180196
{ base: pullRequestModel.base.name, owner: pullRequestModel.remote.owner, repo: pullRequestModel.remote.repositoryName },
181197
pullRequestModel
182-
);
198+
), this._currentDisposables);
183199

184200
this.registerListeners(folderRepoManager.repository, false);
185201

186-
this._disposables.push(
202+
addDisposable(
187203
vscode.window.registerWebviewViewProvider(
188204
this._createPRViewProvider.viewType,
189205
this._createPRViewProvider,
190206
),
207+
this._currentDisposables
191208
);
192209
}
193210

@@ -230,18 +247,19 @@ export class CreatePullRequestHelper implements vscode.Disposable {
230247
pullRequestDefaults,
231248
);
232249

233-
this._treeView = new CompareChanges(
250+
this._treeView = addDisposable(new CompareChanges(
234251
folderRepoManager,
235252
model
236-
);
253+
), this._currentDisposables);
237254

238255
this.registerListeners(folderRepoManager.repository, !compareBranch);
239256

240-
this._disposables.push(
257+
addDisposable(
241258
vscode.window.registerWebviewViewProvider(
242259
this._createPRViewProvider.viewType,
243260
this._createPRViewProvider,
244261
),
262+
this._currentDisposables
245263
);
246264
} else {
247265
createViewProvider = this._createPRViewProvider;
@@ -252,18 +270,16 @@ export class CreatePullRequestHelper implements vscode.Disposable {
252270

253271
private reset() {
254272
this.setActiveContext(false);
255-
this._createPRViewProvider?.dispose();
273+
disposeAll(this._currentDisposables);
256274
this._createPRViewProvider = undefined;
257-
258-
this._treeView?.dispose();
259275
this._treeView = undefined;
260276
this._postCreateCallback = undefined;
261277
this._activeContext = undefined;
262278

263-
disposeAll(this._disposables);
264279
}
265280

266-
dispose() {
281+
override dispose() {
267282
this.reset();
283+
super.dispose();
268284
}
269285
}

src/view/webviewViewCoordinator.ts

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import * as vscode from 'vscode';
7-
import { Disposable } from '../common/lifecycle';
7+
import { addDisposable, Disposable, disposeAll } from '../common/lifecycle';
88
import { PullRequestViewProvider } from '../github/activityBarViewProvider';
99
import { FolderRepositoryManager } from '../github/folderRepositoryManager';
1010
import { PullRequestModel } from '../github/pullRequestModel';
@@ -13,25 +13,31 @@ import { ReviewManager } from './reviewManager';
1313
export class WebviewViewCoordinator extends Disposable {
1414
private _webviewViewProvider?: PullRequestViewProvider;
1515
private _pullRequestModel: Map<PullRequestModel, { folderRepositoryManager: FolderRepositoryManager, reviewManager: ReviewManager }> = new Map();
16+
private readonly _currentDisposables: Disposable[] = [];
1617

1718
constructor(private _context: vscode.ExtensionContext) {
1819
super();
1920
}
2021

21-
override dispose() {
22+
public override dispose() {
2223
super.dispose();
24+
this.reset();
25+
}
26+
27+
reset() {
28+
disposeAll(this._currentDisposables);
2329
this._webviewViewProvider = undefined;
2430
}
2531

2632
private create(pullRequestModel: PullRequestModel, folderRepositoryManager: FolderRepositoryManager, reviewManager: ReviewManager) {
27-
this._webviewViewProvider = this._register(new PullRequestViewProvider(this._context.extensionUri, folderRepositoryManager, reviewManager, pullRequestModel));
28-
this._register(vscode.window.registerWebviewViewProvider(
33+
this._webviewViewProvider = addDisposable(new PullRequestViewProvider(this._context.extensionUri, folderRepositoryManager, reviewManager, pullRequestModel), this._currentDisposables);
34+
addDisposable(vscode.window.registerWebviewViewProvider(
2935
this._webviewViewProvider.viewType,
3036
this._webviewViewProvider,
31-
));
32-
this._register(vscode.commands.registerCommand('pr.refreshActivePullRequest', _ => {
37+
), this._currentDisposables);
38+
addDisposable(vscode.commands.registerCommand('pr.refreshActivePullRequest', _ => {
3339
this._webviewViewProvider?.refresh();
34-
}));
40+
}), this._currentDisposables);
3541
}
3642

3743
public setPullRequest(pullRequestModel: PullRequestModel, folderRepositoryManager: FolderRepositoryManager, reviewManager: ReviewManager, replace?: PullRequestModel) {
@@ -45,7 +51,7 @@ export class WebviewViewCoordinator extends Disposable {
4551
private updatePullRequest() {
4652
const pullRequestModel = Array.from(this._pullRequestModel.keys())[0];
4753
if (!pullRequestModel) {
48-
this.dispose();
54+
this.reset();
4955
return;
5056
}
5157
const { folderRepositoryManager, reviewManager } = this._pullRequestModel.get(pullRequestModel)!;

0 commit comments

Comments
 (0)