refactor(treeview): mr items can be now disposed

src/__mocks__/vscode.js

 module.exports = {
-  TreeItem: jest.fn(),
-  ThemeIcon: jest.fn(),
+  TreeItem: function TreeItem(label, collapsibleState) {
+    return { label, collapsibleState };
+  },
+  ThemeIcon: function ThemeIcon(id) {
+    return { id };
+  },
   EventEmitter: jest.fn(),
   TreeItemCollapsibleState: {
     Collapsed: 'collapsed',

src/constants.ts

 export const GITLAB_COM_URL = 'https://gitlab.com';
 export const REVIEW_URI_SCHEME = 'gl-review';
+export const CONFIG_NAMESPACE = 'gitlab';
+export const CONFIG_CUSTOM_QUERIES = 'customQueries';

src/data_providers/current_branch.js

@@ -4,7 +4,8 @@ const gitLabService = require('../gitlab_service');
 const { ErrorItem } = require('./items/error_item');
 const { getCurrentWorkspaceFolder } = require('../services/workspace_service');
 const { handleError, logError } = require('../log');
-const { MrItem } = require('./items/mr_item');
+const { ItemModel } = require('./items/item_model');
+const { MrItemModel } = require('./items/mr_item_model');
 const { IssueItem } = require('./items/issue_item');
 const { ExternalUrlItem } = require('./items/external_url_item');
 
@@ -16,6 +17,7 @@ class DataProvider {
     this.onDidChangeTreeData = this.eventEmitter.event;
     this.project = null;
     this.mr = null;
+    this.disposableChildren = [];
   }
 
   async fetchPipeline(workspaceFolder) {
@@ -58,7 +60,9 @@ class DataProvider {
     }
     if (mr) {
       this.mr = mr;
-      return new MrItem(this.mr, this.project);
+      const item = new MrItemModel(this.mr, this.project);
+      this.disposableChildren.push(item);
+      return item;
     }
     return new vscode.TreeItem('No merge request found');
   }
@@ -76,6 +80,8 @@ class DataProvider {
 
   async getChildren(item) {
     if (item) return item.getChildren();
+    this.disposableChildren.forEach(s => s.dispose());
+    this.disposableChildren = [];
     try {
       const workspaceFolder = await getCurrentWorkspaceFolder();
       this.project = await gitLabService.fetchCurrentProject(workspaceFolder);
@@ -91,6 +97,7 @@ class DataProvider {
 
   // eslint-disable-next-line class-methods-use-this
   getTreeItem(item) {
+    if (item instanceof ItemModel) return item.getTreeItem();
     return item;
   }
 

src/data_providers/issuable.js

-const vscode = require('vscode');
-const { CustomQueryItem } = require('./items/custom_query_item');
-const { MultirootCustomQueryItem } = require('./items/multiroot_custom_query_item');
-const gitLabService = require('../gitlab_service');
-
-class DataProvider {
-  constructor() {
-    // Temporarily disable eslint to be able to start enforcing stricter rules
-    // eslint-disable-next-line no-underscore-dangle
-    this._onDidChangeTreeData = new vscode.EventEmitter();
-    // Temporarily disable eslint to be able to start enforcing stricter rules
-    // eslint-disable-next-line no-underscore-dangle
-    this.onDidChangeTreeData = this._onDidChangeTreeData.event;
-  }
-
-  // eslint-disable-next-line class-methods-use-this
-  async getChildren(el) {
-    if (el) return el.getChildren(el);
-    const projects = await gitLabService.getAllGitlabProjects();
-    const { customQueries } = vscode.workspace.getConfiguration('gitlab');
-    if (projects.length === 0) return new vscode.TreeItem('No projects found');
-    if (projects.length === 1)
-      return customQueries.map(customQuery => new CustomQueryItem(customQuery, projects[0]));
-    return customQueries.map(customQuery => new MultirootCustomQueryItem(customQuery, projects));
-  }
-
-  // eslint-disable-next-line class-methods-use-this
-  getParent() {
-    return null;
-  }
-
-  // eslint-disable-next-line class-methods-use-this
-  getTreeItem(item) {
-    return item;
-  }
-
-  refresh() {
-    // Temporarily disable eslint to be able to start enforcing stricter rules
-    // eslint-disable-next-line no-underscore-dangle
-    this._onDidChangeTreeData.fire();
-  }
-}
-
-exports.DataProvider = DataProvider;

src/data_providers/issuable.ts

+import * as vscode from 'vscode';
+import { CustomQueryItemModel } from './items/custom_query_item_model';
+import { MultirootCustomQueryItemModel } from './items/multiroot_custom_query_item_model';
+
+import * as gitLabService from '../gitlab_service';
+import { CustomQuery } from '../gitlab/custom_query';
+import { ItemModel } from './items/item_model';
+import { CONFIG_CUSTOM_QUERIES, CONFIG_NAMESPACE } from '../constants';
+
+export class DataProvider implements vscode.TreeDataProvider<ItemModel | vscode.TreeItem> {
+  private eventEmitter = new vscode.EventEmitter<void>();
+
+  private children: ItemModel[] = [];
+
+  onDidChangeTreeData = this.eventEmitter.event;
+
+  async getChildren(el: ItemModel | undefined): Promise<ItemModel[] | vscode.TreeItem[]> {
+    if (el) return el.getChildren();
+
+    this.children.forEach(ch => ch.dispose());
+    this.children = [];
+
+    const projects = await gitLabService.getAllGitlabProjects();
+    if (projects.length === 0) return [new vscode.TreeItem('No projects found')];
+    const customQueries =
+      vscode.workspace
+        .getConfiguration(CONFIG_NAMESPACE)
+        .get<CustomQuery[]>(CONFIG_CUSTOM_QUERIES) || [];
+    if (projects.length === 1) {
+      this.children = customQueries.map(q => new CustomQueryItemModel(q, projects[0]));
+      return this.children;
+    }
+    this.children = customQueries.map(q => new MultirootCustomQueryItemModel(q, projects));
+    return this.children;
+  }
+
+  // eslint-disable-next-line class-methods-use-this
+  getParent() {
+    return null;
+  }
+
+  // eslint-disable-next-line class-methods-use-this
+  getTreeItem(item: vscode.TreeItem | ItemModel) {
+    if (item instanceof ItemModel) return item.getTreeItem();
+    return item;
+  }
+
+  refresh() {
+    this.eventEmitter.fire();
+  }
+}

src/data_providers/items/custom_query_item.test.ts → src/data_providers/items/custom_query_item_model.test.ts

 import * as vscode from 'vscode';
 import { customQuery, project } from '../../test_utils/entities';
 
-import { CustomQueryItem } from './custom_query_item';
+import { CustomQueryItemModel } from './custom_query_item_model';
 
 describe('CustomQueryItem', () => {
-  // eslint-disable-next-line @typescript-eslint/no-unused-vars
-  let item;
+  let item: vscode.TreeItem;
 
   describe('item labeled as a query', () => {
     beforeEach(() => {
-      item = new CustomQueryItem(customQuery, project);
+      item = new CustomQueryItemModel(customQuery, project).getTreeItem();
     });
 
     it('should have query name as label', () => {
-      expect(vscode.TreeItem).toBeCalledWith(
-        'Query name',
-        vscode.TreeItemCollapsibleState.Collapsed,
-      );
+      expect(item.label).toBe('Query name');
     });
 
     it('should have filter icon', () => {
-      expect(vscode.ThemeIcon).toHaveBeenCalledWith('filter');
+      expect(item.iconPath).toEqual(new vscode.ThemeIcon('filter'));
     });
   });
 
   describe('item labeled as a project', () => {
     beforeEach(() => {
-      item = new CustomQueryItem(customQuery, project, true);
+      item = new CustomQueryItemModel(customQuery, project, true).getTreeItem();
     });
 
     it('should have project label as label', () => {
-      expect(vscode.TreeItem).toBeCalledWith(
-        'Project label',
-        vscode.TreeItemCollapsibleState.Collapsed,
-      );
+      expect(item.label).toBe('Project label');
     });
 
     it('should have project icon', () => {
-      expect(vscode.ThemeIcon).toHaveBeenCalledWith('project');
+      expect(item.iconPath).toEqual(new vscode.ThemeIcon('project'));
     });
   });
 });

src/data_providers/items/custom_query_item.ts → src/data_providers/items/custom_query_item_model.ts

@@ -2,27 +2,34 @@ import * as vscode from 'vscode';
 import * as gitLabService from '../../gitlab_service';
 import { handleError } from '../../log';
 import { ErrorItem } from './error_item';
-import { MrItem } from './mr_item';
+import { MrItemModel } from './mr_item_model';
 import { ExternalUrlItem } from './external_url_item';
 import { IssueItem } from './issue_item';
 import { VulnerabilityItem } from './vulnerability_item';
 import { CustomQuery } from '../../gitlab/custom_query';
 import { CustomQueryType } from '../../gitlab/custom_query_type';
+import { ItemModel } from './item_model';
 
-export class CustomQueryItem extends vscode.TreeItem {
+export class CustomQueryItemModel extends ItemModel {
   private project: VsProject;
 
   private customQuery: CustomQuery;
 
-  constructor(customQuery: CustomQuery, project: VsProject, showProject = false) {
-    super(
-      showProject ? project.label : customQuery.name,
-      vscode.TreeItemCollapsibleState.Collapsed,
-    );
-
+  constructor(customQuery: CustomQuery, project: VsProject, readonly showProject = false) {
+    super();
     this.project = project;
     this.customQuery = customQuery;
-    this.iconPath = showProject ? new vscode.ThemeIcon('project') : new vscode.ThemeIcon('filter');
+  }
+
+  getTreeItem(): vscode.TreeItem {
+    const item = new vscode.TreeItem(
+      this.showProject ? this.project.label : this.customQuery.name,
+      vscode.TreeItemCollapsibleState.Collapsed,
+    );
+    item.iconPath = this.showProject
+      ? new vscode.ThemeIcon('project')
+      : new vscode.ThemeIcon('filter');
+    return item;
   }
 
   private async getProjectIssues(): Promise<vscode.TreeItem[]> {
@@ -34,8 +41,11 @@ export class CustomQueryItem extends vscode.TreeItem {
 
     const { MR, ISSUE, SNIPPET, EPIC, VULNERABILITY } = CustomQueryType;
     switch (this.customQuery.type) {
-      case MR:
-        return issues.map((mr: RestIssuable) => new MrItem(mr, this.project));
+      case MR: {
+        const mrModels = issues.map((mr: RestIssuable) => new MrItemModel(mr, this.project));
+        this.setDisposableChildren(mrModels);
+        return mrModels;
+      }
       case ISSUE:
         return issues.map((issue: RestIssuable) => new IssueItem(issue, this.project));
       case SNIPPET:

src/data_providers/items/item_model.ts

+import * as vscode from 'vscode';
+
+export abstract class ItemModel implements vscode.Disposable {
+  private disposableChildren: vscode.Disposable[] = [];
+
+  abstract getTreeItem(): vscode.TreeItem;
+
+  abstract getChildren(): Promise<vscode.TreeItem[] | ItemModel[]>;
+
+  protected setDisposableChildren(children: vscode.Disposable[]): void {
+    this.disposableChildren = children;
+  }
+
+  dispose(): void {
+    this.disposableChildren.forEach(ch => ch.dispose());
+  }
+}

src/data_providers/items/mr_item.ts → src/data_providers/items/mr_item_model.ts

-import { TreeItem, TreeItemCollapsibleState, ThemeIcon, Uri } from 'vscode';
+import * as vscode from 'vscode';
 import { PROGRAMMATIC_COMMANDS } from '../../command_names';
+import { log } from '../../log';
 import { createGitLabNewService } from '../../service_factory';
 import { ChangedFileItem } from './changed_file_item';
+import { ItemModel } from './item_model';
 
-export class MrItem extends TreeItem {
-  mr: RestIssuable;
-
-  project: VsProject;
+export class MrItemModel extends ItemModel {
+  constructor(readonly mr: RestIssuable, readonly project: VsProject) {
+    super();
+  }
 
-  constructor(mr: RestIssuable, project: VsProject) {
-    super(`!${mr.iid} · ${mr.title}`, TreeItemCollapsibleState.Collapsed);
-    this.mr = mr;
-    this.project = project;
-    this.iconPath = Uri.parse(mr.author.avatar_url);
+  getTreeItem(): vscode.TreeItem {
+    const { iid, title, author } = this.mr;
+    const item = new vscode.TreeItem(
+      `!${iid} · ${title}`,
+      vscode.TreeItemCollapsibleState.Collapsed,
+    );
+    item.iconPath = vscode.Uri.parse(author.avatar_url);
+    return item;
   }
 
-  async getChildren(): Promise<TreeItem[]> {
-    const description = new TreeItem('Description');
-    description.iconPath = new ThemeIcon('note');
+  async getChildren(): Promise<vscode.TreeItem[]> {
+    const description = new vscode.TreeItem('Description');
+    description.iconPath = new vscode.ThemeIcon('note');
     description.command = {
       command: PROGRAMMATIC_COMMANDS.SHOW_RICH_CONTENT,
       arguments: [this.mr, this.project.uri],
@@ -27,9 +32,13 @@ export class MrItem extends TreeItem {
     return [description, ...changedFiles];
   }
 
-  private async getChangedFiles(): Promise<TreeItem[]> {
+  private async getChangedFiles(): Promise<vscode.TreeItem[]> {
     const gitlabService = await createGitLabNewService(this.project.uri);
     const mrVersion = await gitlabService.getMrDiff(this.mr);
     return mrVersion.diffs.map(d => new ChangedFileItem(this.mr, mrVersion, d, this.project));
   }
+
+  dispose(): void {
+    log(`MR ${this.mr.title} item model got disposed`);
+  }
 }

src/data_providers/items/multiroot_custom_query_item.js

-const vscode = require('vscode');
-const { CustomQueryItem } = require('./custom_query_item');
-
-class MultirootCustomQueryItem extends vscode.TreeItem {
-  constructor(customQuery, projects) {
-    super(customQuery.name, vscode.TreeItemCollapsibleState.Collapsed);
-
-    this.customQuery = customQuery;
-    this.projects = projects;
-
-    this.iconPath = new vscode.ThemeIcon('filter');
-  }
-
-  async getChildren() {
-    return this.projects.map(p => new CustomQueryItem(this.customQuery, p, true));
-  }
-}
-
-exports.MultirootCustomQueryItem = MultirootCustomQueryItem;

src/data_providers/items/multiroot_custom_query_item.test.js

-jest.mock('./custom_query_item');
-
-const vscode = require('vscode');
-
-const { MultirootCustomQueryItem } = require('./multiroot_custom_query_item');
-const { CustomQueryItem } = require('./custom_query_item');
-
-describe('MultirootCustomQueryItem', () => {
-  const customQuery = { name: 'Query name' };
-
-  let item;
-
-  beforeEach(() => {
-    const projects = ['a', 'b'];
-    item = new MultirootCustomQueryItem(customQuery, projects);
-  });
-
-  it('should use query name to create collapsed item', () => {
-    expect(vscode.TreeItem).toBeCalledWith('Query name', vscode.TreeItemCollapsibleState.Collapsed);
-  });
-
-  it('should return custom query children', async () => {
-    CustomQueryItem.mockImplementation((query, project, showProject) => ({
-      query,
-      project,
-      showProject,
-    }));
-    const [a, b] = await item.getChildren();
-    expect(a).toEqual({ query: customQuery, project: 'a', showProject: true });
-    expect(b).toEqual({ query: customQuery, project: 'b', showProject: true });
-  });
-});

src/data_providers/items/multiroot_custom_query_item_model.test.ts

+import * as vscode from 'vscode';
+import { MultirootCustomQueryItemModel } from './multiroot_custom_query_item_model';
+import { CustomQueryItemModel } from './custom_query_item_model';
+import { customQuery, project } from '../../test_utils/entities';
+
+const projects = [
+  { ...project, label: 'label p1' },
+  { ...project, label: 'label p2' },
+];
+
+describe('MultirootCustomQueryItem', () => {
+  let item: MultirootCustomQueryItemModel;
+
+  beforeEach(() => {
+    item = new MultirootCustomQueryItemModel(customQuery, projects);
+  });
+
+  it('should use query name to create collapsed item', async () => {
+    const treeItem = await item.getTreeItem();
+    expect(treeItem.label).toBe('Query name');
+    expect(treeItem.collapsibleState).toBe(vscode.TreeItemCollapsibleState.Collapsed);
+  });
+
+  it('should return custom query children with project label', async () => {
+    const [a, b] = await item.getChildren();
+    expect(a).toBeInstanceOf(CustomQueryItemModel);
+    expect(b).toBeInstanceOf(CustomQueryItemModel);
+    expect(await a.getTreeItem().label).toBe('label p1');
+    expect(await b.getTreeItem().label).toBe('label p2');
+  });
+});

src/data_providers/items/multiroot_custom_query_item_model.ts

+import * as vscode from 'vscode';
+import { CustomQueryItemModel } from './custom_query_item_model';
+import { CustomQuery } from '../../gitlab/custom_query';
+import { ItemModel } from './item_model';
+
+export class MultirootCustomQueryItemModel extends ItemModel {
+  private projects: VsProject[];
+
+  private customQuery: CustomQuery;
+
+  constructor(customQuery: CustomQuery, projects: VsProject[]) {
+    super();
+    this.customQuery = customQuery;
+    this.projects = projects;
+  }
+
+  getTreeItem(): vscode.TreeItem {
+    const item = new vscode.TreeItem(
+      this.customQuery.name,
+      vscode.TreeItemCollapsibleState.Collapsed,
+    );
+    item.iconPath = new vscode.ThemeIcon('filter');
+    return item;
+  }
+
+  async getChildren(): Promise<ItemModel[]> {
+    const queryModels = this.projects.map(p => new CustomQueryItemModel(this.customQuery, p, true));
+    this.setDisposableChildren(queryModels);
+    return queryModels;
+  }
+}

src/gitlab_service.ts

@@ -199,7 +199,7 @@ export async function fetchVersion() {
   return versionCache;
 }
 
-export async function getAllGitlabProjects() {
+export async function getAllGitlabProjects(): Promise<VsProject[]> {
   if (!vscode.workspace.workspaceFolders) {
     return [];
   }
@@ -209,8 +209,7 @@ export async function getAllGitlabProjects() {
   }));
 
   const fetchedProjectsWithUri = await Promise.all(projectsWithUri);
-
-  return fetchedProjectsWithUri.filter(p => p.label);
+  return fetchedProjectsWithUri.filter((p): p is VsProject => Boolean(p.label));
 }
 
 export async function fetchLastPipelineForCurrentBranch(workspaceFolder: string) {

test/integration/for_current_branch.test.js

@@ -47,7 +47,7 @@ describe('GitLab tree view for current branch', () => {
     server = getServer([pipelinesEndpoint, pipelineEndpoint, mrEndpoint, issueEndpoint]);
     const forCurrentBranch = await dataProvider.getChildren();
     assert.deepStrictEqual(
-      forCurrentBranch.map(i => i.label),
+      forCurrentBranch.map(i => dataProvider.getTreeItem(i).label),
       [
         'Pipeline #47 passed · Finished 4 years ago',
         '!33824 · Web IDE - remove unused actions (mappings)',
@@ -60,7 +60,7 @@ describe('GitLab tree view for current branch', () => {
     server = getServer([mrEndpoint, issueEndpoint]);
     const forCurrentBranch = await dataProvider.getChildren();
     assert.deepStrictEqual(
-      forCurrentBranch.map(i => i.label),
+      forCurrentBranch.map(i => dataProvider.getTreeItem(i).label),
       [
         'Fetching pipeline failed',
         '!33824 · Web IDE - remove unused actions (mappings)',
@@ -73,7 +73,7 @@ describe('GitLab tree view for current branch', () => {
     server = getServer([pipelinesEndpoint, pipelineEndpoint]);
     const forCurrentBranch = await dataProvider.getChildren();
     assert.deepStrictEqual(
-      forCurrentBranch.map(i => i.label),
+      forCurrentBranch.map(i => dataProvider.getTreeItem(i).label),
       [
         'Pipeline #47 passed · Finished 4 years ago',
         'Fetching MR failed',
@@ -86,7 +86,7 @@ describe('GitLab tree view for current branch', () => {
     server = getServer([pipelinesEndpoint, pipelineEndpoint, mrEndpoint]);
     const forCurrentBranch = await dataProvider.getChildren();
     assert.deepStrictEqual(
-      forCurrentBranch.map(i => i.label),
+      forCurrentBranch.map(i => dataProvider.getTreeItem(i).label),
       [
         'Pipeline #47 passed · Finished 4 years ago',
         '!33824 · Web IDE - remove unused actions (mappings)',

test/integration/tree_view.test.js

@@ -114,15 +114,19 @@ describe('GitLab tree view', () => {
     await vscode.workspace.getConfiguration().update('gitlab.customQueries', undefined);
   });
 
+  const getTreeItem = model => dataProvider.getTreeItem(model);
+
   /**
    * Opens a top level category from the extension issues tree view
    */
   async function openCategory(label) {
     const categories = await dataProvider.getChildren();
-    const [chosenCategory] = categories.filter(c => c.label === label);
+    const [chosenCategory] = categories.filter(c => getTreeItem(c).label === label);
     assert(
       chosenCategory,
-      `Can't open category ${label} because it's not present in ${categories.map(c => c.label)}`,
+      `Can't open category ${label} because it's not present in ${categories.map(
+        c => getTreeItem(c).label,
+      )}`,
     );
     return await dataProvider.getChildren(chosenCategory);
   }
@@ -141,19 +145,20 @@ describe('GitLab tree view', () => {
     const mergeRequestsAssignedToMe = await openCategory('Merge requests assigned to me');
 
     assert.strictEqual(mergeRequestsAssignedToMe.length, 1);
-    const mrItem = mergeRequestsAssignedToMe[0];
+    const mrItemModel = mergeRequestsAssignedToMe[0];
+    const mrItem = getTreeItem(mrItemModel);
     assert.strictEqual(mrItem.label, '!33824 · Web IDE - remove unused actions (mappings)');
     assert.strictEqual(
       mrItem.iconPath.toString(true),
       'https://secure.gravatar.com/avatar/6042a9152ada74d9fb6a0cdce895337e?s=80&d=identicon',
     );
 
-    const mrContent = await dataProvider.getChildren(mrItem);
-    assert.strictEqual(mrContent[0].label, 'Description');
+    const mrContent = await dataProvider.getChildren(mrItemModel);
+    assert.strictEqual(getTreeItem(mrContent[0]).label, 'Description');
 
     const mrFiles = mrContent.slice(1);
     assert.deepStrictEqual(
-      mrFiles.map(f => f.resourceUri.path),
+      mrFiles.map(f => getTreeItem(f).resourceUri.path),
       [
         '/.deleted.yml',
         '/README1.md',
@@ -164,7 +169,7 @@ describe('GitLab tree view', () => {
       ],
     );
     assert.deepStrictEqual(
-      mrFiles.map(f => f.description),
+      mrFiles.map(f => getTreeItem(f).description),
       ['[deleted] /', '[renamed] /', '[added] /', '/src', '[added] /src/assets', '[renamed] /'],
     );
   });
@@ -183,10 +188,13 @@ describe('GitLab tree view', () => {
       const mergeRequestsAssignedToMe = await openCategory('Merge requests assigned to me');
 
       assert.strictEqual(mergeRequestsAssignedToMe.length, 1);
-      const mrItem = mergeRequestsAssignedToMe[0];
-      assert.strictEqual(mrItem.label, '!33824 · Web IDE - remove unused actions (mappings)');
+      const mrModel = mergeRequestsAssignedToMe[0];
+      assert.strictEqual(
+        getTreeItem(mrModel).label,
+        '!33824 · Web IDE - remove unused actions (mappings)',
+      );
 
-      const mrContent = await dataProvider.getChildren(mrItem);
+      const mrContent = await dataProvider.getChildren(mrModel);
       assert.strictEqual(mrContent[0].label, 'Description');
 
       mrFiles = mrContent.slice(1);
@@ -244,13 +252,13 @@ describe('GitLab tree view', () => {
     const customMergeRequests = await openCategory('Custom GitLab Query for MR');
 
     assert.strictEqual(customMergeRequests.length, 1);
-    assert.strictEqual(customMergeRequests[0].label, '!33824 · Custom Query MR');
+    assert.strictEqual(getTreeItem(customMergeRequests[0]).label, '!33824 · Custom Query MR');
   });
 
   it('handles full custom query for issues', async () => {
     const customMergeRequests = await openCategory('Custom GitLab Query for issues');
 
     assert.strictEqual(customMergeRequests.length, 1);
-    assert.strictEqual(customMergeRequests[0].label, '#219925 · Custom Query Issue');
+    assert.strictEqual(getTreeItem(customMergeRequests[0]).label, '#219925 · Custom Query Issue');
   });
 });