Part 2 - Make Model view components disposable + dispose all emitters (#24097)

This commit is contained in:
Cheena Malhotra
2023-08-23 12:46:09 -07:00
committed by GitHub
parent 25a658775c
commit 6c6464e772
58 changed files with 435 additions and 195 deletions

View File

@@ -20,6 +20,7 @@ import { IExtensionDescription } from 'vs/platform/extensions/common/extensions'
import { ILogService } from 'vs/platform/log/common/log';
import { onUnexpectedError } from 'vs/base/common/errors';
import { SqlMainContext } from 'vs/workbench/api/common/extHost.protocol';
import { Disposable } from 'vs/base/common/lifecycle';
class ModelBuilderImpl implements azdata.ModelBuilder {
private nextComponentId: number;
@@ -594,16 +595,16 @@ class InternalItemConfig {
}
}
class ComponentWrapper implements azdata.Component {
class ComponentWrapper extends Disposable implements azdata.Component {
public properties: { [key: string]: any } = {};
public layout: any;
public itemConfigs: InternalItemConfig[];
public customValidations: ((component: ThisType<ComponentWrapper>) => boolean | Thenable<boolean>)[] = [];
private _valid: boolean = true;
private _onValidityChangedEmitter = new Emitter<boolean>();
private _onValidityChangedEmitter = this._register(new Emitter<boolean>());
public readonly onValidityChanged = this._onValidityChangedEmitter.event;
private _onErrorEmitter = new Emitter<Error>();
private _onErrorEmitter = this._register(new Emitter<Error>());
public readonly onError: vscode.Event<Error> = this._onErrorEmitter.event;
protected _emitterMap = new Map<ComponentEventType, Emitter<any>>();
@@ -613,10 +614,17 @@ class ComponentWrapper implements azdata.Component {
protected _id: string,
protected _logService: ILogService
) {
super();
this.properties = {};
this.itemConfigs = [];
}
public getRegisteredEmitter<T>(): Emitter<T> {
let emitter = new Emitter<T>();
this._register(emitter);
return emitter;
}
public get id(): string {
return this._id;
}
@@ -729,6 +737,7 @@ class ComponentWrapper implements azdata.Component {
this._logService.warn(`Trying to add duplicate component ${item.id} to container ${this.id}`);
return false;
}
this._register(item);
return true;
});
if (items.length === 0) {
@@ -769,6 +778,7 @@ class ComponentWrapper implements azdata.Component {
this._logService.warn(`Trying to add duplicate component ${item.id} to container ${this.id}`);
return;
}
this._register(item);
const config = this.createAndAddItemConfig(item, itemLayout, index);
this._proxy.$addToContainer(this._handle, this.id, [{ itemConfig: config.toIItemConfig(), index }]).then(undefined, (err) => this.handleError(err));
}
@@ -784,6 +794,7 @@ class ComponentWrapper implements azdata.Component {
if (!itemImpl) {
throw new Error(nls.localize('unknownComponentType', "Unknown component type. Must use ModelBuilder to create objects"));
}
this._register(itemImpl);
const config = new InternalItemConfig(itemImpl, itemLayout);
if (index !== undefined && index >= 0 && index <= this.items.length) {
this.itemConfigs.splice(index, 0, config);
@@ -932,7 +943,7 @@ class CardWrapper extends ComponentWrapper implements azdata.CardComponent {
constructor(proxy: MainThreadModelViewShape, handle: number, id: string, logService: ILogService) {
super(proxy, handle, ModelComponentTypes.Card, id, logService);
this.properties = {};
this._emitterMap.set(ComponentEventType.onDidClick, new Emitter<any>());
this._emitterMap.set(ComponentEventType.onDidClick, this.getRegisteredEmitter<any>());
}
public get label(): string {
@@ -1001,8 +1012,8 @@ class InputBoxWrapper extends ComponentWrapper implements azdata.InputBoxCompone
constructor(proxy: MainThreadModelViewShape, handle: number, id: string, logService: ILogService) {
super(proxy, handle, ModelComponentTypes.InputBox, id, logService);
this.properties = {};
this._emitterMap.set(ComponentEventType.onDidChange, new Emitter<any>());
this._emitterMap.set(ComponentEventType.onEnterKeyPressed, new Emitter<string>());
this._emitterMap.set(ComponentEventType.onDidChange, this.getRegisteredEmitter<any>());
this._emitterMap.set(ComponentEventType.onEnterKeyPressed, this.getRegisteredEmitter<string>());
}
public get value(): string {
@@ -1113,7 +1124,7 @@ class CheckBoxWrapper extends ComponentWrapper implements azdata.CheckBoxCompone
constructor(proxy: MainThreadModelViewShape, handle: number, id: string, logService: ILogService) {
super(proxy, handle, ModelComponentTypes.CheckBox, id, logService);
this.properties = {};
this._emitterMap.set(ComponentEventType.onDidChange, new Emitter<any>());
this._emitterMap.set(ComponentEventType.onDidChange, this.getRegisteredEmitter<any>());
}
public get checked(): boolean {
@@ -1142,7 +1153,7 @@ class WebViewWrapper extends ComponentWrapper implements azdata.WebViewComponent
this.properties = {
'extensionLocation': this._extensionLocation
};
this._emitterMap.set(ComponentEventType.onMessage, new Emitter<any>());
this._emitterMap.set(ComponentEventType.onMessage, this.getRegisteredEmitter<any>());
}
public get message(): any {
@@ -1176,8 +1187,8 @@ class EditorWrapper extends ComponentWrapper implements azdata.EditorComponent {
constructor(proxy: MainThreadModelViewShape, handle: number, id: string, logService: ILogService) {
super(proxy, handle, ModelComponentTypes.Editor, id, logService);
this.properties = {};
this._emitterMap.set(ComponentEventType.onDidChange, new Emitter<any>());
this._emitterMap.set(ComponentEventType.onComponentCreated, new Emitter<any>());
this._emitterMap.set(ComponentEventType.onDidChange, this.getRegisteredEmitter<any>());
this._emitterMap.set(ComponentEventType.onComponentCreated, this.getRegisteredEmitter<any>());
}
public get content(): string {
@@ -1229,8 +1240,8 @@ class DiffEditorWrapper extends ComponentWrapper implements azdata.DiffEditorCom
constructor(proxy: MainThreadModelViewShape, handle: number, id: string, logService: ILogService) {
super(proxy, handle, ModelComponentTypes.DiffEditor, id, logService);
this.properties = {};
this._emitterMap.set(ComponentEventType.onDidChange, new Emitter<any>());
this._emitterMap.set(ComponentEventType.onComponentCreated, new Emitter<any>());
this._emitterMap.set(ComponentEventType.onDidChange, this.getRegisteredEmitter<any>());
this._emitterMap.set(ComponentEventType.onComponentCreated, this.getRegisteredEmitter<any>());
}
public get contentLeft(): string {
@@ -1316,8 +1327,8 @@ class RadioButtonWrapper extends ComponentWrapper implements azdata.RadioButtonC
constructor(proxy: MainThreadModelViewShape, handle: number, id: string, logService: ILogService) {
super(proxy, handle, ModelComponentTypes.RadioButton, id, logService);
this.properties = {};
this._emitterMap.set(ComponentEventType.onDidClick, new Emitter<any>());
this._emitterMap.set(ComponentEventType.onDidChange, new Emitter<boolean>());
this._emitterMap.set(ComponentEventType.onDidClick, this.getRegisteredEmitter<any>());
this._emitterMap.set(ComponentEventType.onDidChange, this.getRegisteredEmitter<boolean>());
}
public get name(): string {
@@ -1422,8 +1433,8 @@ class TableComponentWrapper extends ComponentWrapper implements azdata.TableComp
constructor(proxy: MainThreadModelViewShape, handle: number, id: string, logService: ILogService) {
super(proxy, handle, ModelComponentTypes.Table, id, logService);
this.properties = {};
this._emitterMap.set(ComponentEventType.onSelectedRowChanged, new Emitter<any>());
this._emitterMap.set(ComponentEventType.onCellAction, new Emitter<any>());
this._emitterMap.set(ComponentEventType.onSelectedRowChanged, this.getRegisteredEmitter<any>());
this._emitterMap.set(ComponentEventType.onCellAction, this.getRegisteredEmitter<any>());
}
public get data(): any[][] {
@@ -1522,7 +1533,7 @@ class DropDownWrapper extends ComponentWrapper implements azdata.DropDownCompone
constructor(proxy: MainThreadModelViewShape, handle: number, id: string, logService: ILogService) {
super(proxy, handle, ModelComponentTypes.DropDown, id, logService);
this.properties = {};
this._emitterMap.set(ComponentEventType.onDidChange, new Emitter<any>());
this._emitterMap.set(ComponentEventType.onDidChange, this.getRegisteredEmitter<any>());
}
public get value(): string | azdata.CategoryValue {
@@ -1600,8 +1611,8 @@ class DeclarativeTableWrapper extends ComponentWrapper implements azdata.Declara
constructor(proxy: MainThreadModelViewShape, handle: number, id: string, logService: ILogService) {
super(proxy, handle, ModelComponentTypes.DeclarativeTable, id, logService);
this.properties = {};
this._emitterMap.set(ComponentEventType.onDidChange, new Emitter<any>());
this._emitterMap.set(ComponentEventType.onSelectedRowChanged, new Emitter<azdata.DeclarativeTableRowSelectedEvent>());
this._emitterMap.set(ComponentEventType.onDidChange, this.getRegisteredEmitter<any>());
this._emitterMap.set(ComponentEventType.onSelectedRowChanged, this.getRegisteredEmitter<azdata.DeclarativeTableRowSelectedEvent>());
}
@@ -1736,7 +1747,7 @@ class ListBoxWrapper extends ComponentWrapper implements azdata.ListBoxComponent
constructor(proxy: MainThreadModelViewShape, handle: number, id: string, logService: ILogService) {
super(proxy, handle, ModelComponentTypes.ListBox, id, logService);
this.properties = {};
this._emitterMap.set(ComponentEventType.onSelectedRowChanged, new Emitter<any>());
this._emitterMap.set(ComponentEventType.onSelectedRowChanged, this.getRegisteredEmitter<any>());
}
public get selectedRow(): number {
@@ -1764,7 +1775,7 @@ class ButtonWrapper extends ComponentWithIconWrapper implements azdata.ButtonCom
constructor(proxy: MainThreadModelViewShape, handle: number, id: string, logService: ILogService) {
super(proxy, handle, ModelComponentTypes.Button, id, logService);
this.properties = {};
this._emitterMap.set(ComponentEventType.onDidClick, new Emitter<any>());
this._emitterMap.set(ComponentEventType.onDidClick, this.getRegisteredEmitter<any>());
}
public get label(): string {
@@ -1841,7 +1852,7 @@ class FileBrowserTreeComponentWrapper extends ComponentWrapper implements azdata
constructor(proxy: MainThreadModelViewShape, handle: number, id: string, logService: ILogService) {
super(proxy, handle, ModelComponentTypes.FileBrowserTree, id, logService);
this.properties = {};
this._emitterMap.set(ComponentEventType.onDidChange, new Emitter<any>());
this._emitterMap.set(ComponentEventType.onDidChange, this.getRegisteredEmitter<any>());
}
public get ownerUri(): string {
@@ -1868,7 +1879,7 @@ class DivContainerWrapper extends ComponentWrapper implements azdata.DivContaine
constructor(proxy: MainThreadModelViewShape, handle: number, type: ModelComponentTypes, id: string, logService: ILogService) {
super(proxy, handle, type, id, logService);
this.properties = {};
this._emitterMap.set(ComponentEventType.onDidClick, new Emitter<any>());
this._emitterMap.set(ComponentEventType.onDidClick, this.getRegisteredEmitter<any>());
}
public get overflowY(): string {
@@ -1920,7 +1931,7 @@ class HyperlinkComponentWrapper extends ComponentWrapper implements azdata.Hyper
constructor(proxy: MainThreadModelViewShape, handle: number, id: string, logService: ILogService) {
super(proxy, handle, ModelComponentTypes.Hyperlink, id, logService);
this.properties = {};
this._emitterMap.set(ComponentEventType.onDidClick, new Emitter<any>());
this._emitterMap.set(ComponentEventType.onDidClick, this.getRegisteredEmitter<any>());
}
public get label(): string {
@@ -1948,8 +1959,8 @@ class RadioCardGroupComponentWrapper extends ComponentWrapper implements azdata.
super(proxy, handle, ModelComponentTypes.RadioCardGroup, id, logService);
this.properties = {};
this._emitterMap.set(ComponentEventType.onDidChange, new Emitter<azdata.RadioCardSelectionChangedEvent>());
this._emitterMap.set(ComponentEventType.onDidClick, new Emitter<azdata.RadioCardLinkClickEvent>());
this._emitterMap.set(ComponentEventType.onDidChange, this.getRegisteredEmitter<azdata.RadioCardSelectionChangedEvent>());
this._emitterMap.set(ComponentEventType.onDidClick, this.getRegisteredEmitter<azdata.RadioCardLinkClickEvent>());
}
public get iconWidth(): string | undefined {
@@ -2023,7 +2034,7 @@ class ListViewComponentWrapper extends ComponentWrapper implements azdata.ListVi
super(proxy, handle, ModelComponentTypes.ListView, id, logService);
this.properties = {};
this._emitterMap.set(ComponentEventType.onDidClick, new Emitter<azdata.ListViewClickEvent>());
this._emitterMap.set(ComponentEventType.onDidClick, this.getRegisteredEmitter<azdata.ListViewClickEvent>());
}
public get title(): azdata.ListViewTitle {
@@ -2059,7 +2070,7 @@ class TabbedPanelComponentWrapper extends ComponentWrapper implements azdata.Tab
constructor(proxy: MainThreadModelViewShape, handle: number, id: string, logService: ILogService) {
super(proxy, handle, ModelComponentTypes.TabbedPanel, id, logService);
this.properties = {};
this._emitterMap.set(ComponentEventType.onDidChange, new Emitter<string>());
this._emitterMap.set(ComponentEventType.onDidChange, this.getRegisteredEmitter<string>());
}
updateTabs(tabs: (azdata.Tab | azdata.TabGroup)[]): void {
@@ -2112,8 +2123,8 @@ class InfoBoxComponentWrapper extends ComponentWrapper implements azdata.InfoBox
constructor(proxy: MainThreadModelViewShape, handle: number, id: string, logService: ILogService) {
super(proxy, handle, ModelComponentTypes.InfoBox, id, logService);
this.properties = {};
this._emitterMap.set(ComponentEventType.onDidClick, new Emitter<any>());
this._emitterMap.set(ComponentEventType.onChildClick, new Emitter<any>());
this._emitterMap.set(ComponentEventType.onDidClick, this.getRegisteredEmitter<any>());
this._emitterMap.set(ComponentEventType.onChildClick, this.getRegisteredEmitter<any>());
}
public get style(): azdata.InfoBoxStyle {
@@ -2179,8 +2190,8 @@ class SliderComponentWrapper extends ComponentWrapper implements azdata.SliderCo
constructor(proxy: MainThreadModelViewShape, handle: number, id: string, logService: ILogService) {
super(proxy, handle, ModelComponentTypes.Slider, id, logService);
this.properties = {};
this._emitterMap.set(ComponentEventType.onDidChange, new Emitter<number>());
this._emitterMap.set(ComponentEventType.onInput, new Emitter<number>());
this._emitterMap.set(ComponentEventType.onDidChange, this.getRegisteredEmitter<number>());
this._emitterMap.set(ComponentEventType.onInput, this.getRegisteredEmitter<number>());
}
public get min(): number | undefined {
@@ -2262,10 +2273,10 @@ class GroupContainerComponentWrapper extends ComponentWrapper implements azdata.
}
}
class ModelViewImpl implements azdata.ModelView {
class ModelViewImpl extends Disposable implements azdata.ModelView {
public onClosedEmitter = new Emitter<any>();
private _onValidityChangedEmitter = new Emitter<boolean>();
public onClosedEmitter = this._register(new Emitter<any>());
private _onValidityChangedEmitter = this._register(new Emitter<boolean>());
public readonly onValidityChanged = this._onValidityChangedEmitter.event;
private _modelBuilder: ModelBuilderImpl;
@@ -2280,6 +2291,7 @@ class ModelViewImpl implements azdata.ModelView {
_extension: IExtensionDescription,
logService: ILogService
) {
super();
this._modelBuilder = new ModelBuilderImpl(this._proxy, this._handle, this._extHostModelViewTree, _extension, logService);
}
@@ -2328,7 +2340,6 @@ class ModelViewImpl implements azdata.ModelView {
export class ExtHostModelView implements ExtHostModelViewShape {
private readonly _proxy: MainThreadModelViewShape;
private readonly _modelViews = new Map<number, ModelViewImpl>();
private readonly _handlers = new Map<string, (view: azdata.ModelView) => void>();
private readonly _handlerToExtension = new Map<string, IExtensionDescription>();

View File

@@ -15,6 +15,9 @@ import { ExtHostModelViewDialogShape, MainThreadModelViewDialogShape, ExtHostMod
import { IExtensionDescription } from 'vs/platform/extensions/common/extensions';
import { TabOrientation, DialogWidth, DialogStyle, DialogPosition, IDialogProperties } from 'sql/workbench/api/common/sqlExtHostTypes';
import { SqlMainContext } from 'vs/workbench/api/common/extHost.protocol';
import { Disposable } from 'vs/base/common/lifecycle';
import { DashboardTab } from 'azdata';
import { DashboardTabGroup } from 'azdata';
const DONE_LABEL = nls.localize('dialogDoneLabel', "Done");
const CANCEL_LABEL = nls.localize('dialogCancelLabel', "Cancel");
@@ -22,7 +25,7 @@ const GENERATE_SCRIPT_LABEL = nls.localize('generateScriptLabel', "Generate scri
const NEXT_LABEL = nls.localize('dialogNextLabel', "Next");
const PREVIOUS_LABEL = nls.localize('dialogPreviousLabel', "Previous");
class ModelViewPanelImpl implements azdata.window.ModelViewPanel {
class ModelViewPanelImpl extends Disposable implements azdata.window.ModelViewPanel {
private _modelView: azdata.ModelView;
public handle: number;
protected _modelViewId: string;
@@ -33,6 +36,7 @@ class ModelViewPanelImpl implements azdata.window.ModelViewPanel {
protected _extHostModelViewDialog: ExtHostModelViewDialog,
protected _extHostModelView: ExtHostModelViewShape,
protected _extension: IExtensionDescription) {
super();
this._onValidityChanged = this._extHostModelViewDialog.getValidityChangedEvent(this);
this._onValidityChanged(valid => this._valid = valid);
}
@@ -43,6 +47,7 @@ class ModelViewPanelImpl implements azdata.window.ModelViewPanel {
this.setModelViewId(viewId);
this._extHostModelView.$registerProvider(viewId, modelView => {
this._modelView = modelView;
this._register(this._modelView);
handler(modelView);
}, this._extension);
}
@@ -460,6 +465,10 @@ export interface WizardPageEventInfo {
pages?: azdata.window.WizardPage[];
}
interface DisposableTab extends azdata.Tab, vscode.Disposable { }
interface DisposableTabGroup extends azdata.TabGroup, vscode.Disposable { }
class WizardImpl implements azdata.window.Wizard {
private _currentPage: number = undefined;
public pages: azdata.window.WizardPage[] = [];
@@ -600,7 +609,7 @@ class WizardImpl implements azdata.window.Wizard {
}
}
class ModelViewDashboardImpl implements azdata.window.ModelViewDashboard {
class ModelViewDashboardImpl extends Disposable implements azdata.window.ModelViewDashboard {
private _tabbedPanel: azdata.TabbedPanelComponent;
private _view: azdata.ModelView;
@@ -608,9 +617,11 @@ class ModelViewDashboardImpl implements azdata.window.ModelViewDashboard {
private _editor: ModelViewEditorImpl,
private _options?: azdata.ModelViewDashboardOptions
) {
super();
this._register(this._editor);
}
updateTabs(tabs: (azdata.DashboardTab | azdata.DashboardTabGroup)[]): void {
updateTabs(tabs: (DashboardTab | DashboardTabGroup)[]): void {
if (this._tabbedPanel === undefined || this._view === undefined) {
throw new Error(nls.localize('dashboardNotInitialized', "Tabs are not initialized"));
}
@@ -618,7 +629,7 @@ class ModelViewDashboardImpl implements azdata.window.ModelViewDashboard {
this._tabbedPanel.updateTabs(this.createTabs(tabs, this._view));
}
registerTabs(handler: (view: azdata.ModelView) => Thenable<(azdata.DashboardTab | azdata.DashboardTabGroup)[]>): void {
registerTabs(handler: (view: azdata.ModelView) => Thenable<(DashboardTab | DashboardTabGroup)[]>): void {
this._editor.registerContent(async (view) => {
this._view = view;
const dashboardTabs = await handler(view);
@@ -628,6 +639,7 @@ class ModelViewDashboardImpl implements azdata.window.ModelViewDashboard {
showIcon: this._options?.showIcon ?? true,
alwaysShowTabs: this._options?.alwaysShowTabs ?? false
}).component();
this._register(this._tabbedPanel);
return view.initializeModel(this._tabbedPanel);
});
}
@@ -640,31 +652,46 @@ class ModelViewDashboardImpl implements azdata.window.ModelViewDashboard {
return this._editor.closeEditor();
}
createTab(tab: azdata.DashboardTab, view: azdata.ModelView): azdata.Tab {
if (tab.toolbar) {
createTab(dashboardTab: DashboardTab, view: azdata.ModelView): DisposableTab {
let tab: DisposableTab;
if (dashboardTab.toolbar) {
const flexContainer = view.modelBuilder.flexContainer().withLayout({ flexFlow: 'column' }).component();
flexContainer.addItem(tab.toolbar, { flex: '0 0 auto' });
flexContainer.addItem(tab.content, { flex: '1 1 auto' });
return {
title: tab.title,
id: tab.id,
flexContainer.addItem(dashboardTab.toolbar, { flex: '0 0 auto' });
flexContainer.addItem(dashboardTab.content, { flex: '1 1 auto' });
tab = {
title: dashboardTab.title,
id: dashboardTab.id,
content: flexContainer,
icon: tab.icon
icon: dashboardTab.icon,
dispose: () => flexContainer.dispose(),
};
} else {
return tab;
let content = dashboardTab.content;
tab = {
content: content,
id: dashboardTab.id,
title: dashboardTab.title,
icon: dashboardTab.icon,
dispose: () => content.dispose()
}
}
this._register(tab);
return tab;
}
createTabs(dashboardTabs: (azdata.DashboardTab | azdata.DashboardTabGroup)[], view: azdata.ModelView): (azdata.TabGroup | azdata.Tab)[] {
const tabs: (azdata.TabGroup | azdata.Tab)[] = [];
dashboardTabs.forEach((item: azdata.DashboardTab | azdata.DashboardTabGroup) => {
createTabs(dashboardTabs: (DashboardTab | DashboardTabGroup)[], view: azdata.ModelView): (DisposableTabGroup | DisposableTab)[] {
const tabs: (DisposableTabGroup | DisposableTab)[] = [];
dashboardTabs.forEach((item: DashboardTab | DashboardTabGroup) => {
if ('tabs' in item) {
tabs.push(<azdata.TabGroup>{
let disposableTabs = item.tabs.map(tab => {
return this.createTab(tab, view);
});
tabs.push(<DisposableTabGroup>{
title: item.title,
tabs: item.tabs.map(tab => {
return this.createTab(tab, view);
})
tabs: disposableTabs,
dispose: () => {
disposableTabs.forEach(t => t.dispose());
},
});
} else {
tabs.push(this.createTab(item, view));

View File

@@ -387,6 +387,8 @@ suite('ExtHostModelView Validation Tests', () => {
rootComponent.properties.data &&
rootComponent.properties.data.length === 0;
})), Times.once());
declarativeTable.dispose();
});
test('initialized with string data has correct properties', async () => {
@@ -401,6 +403,8 @@ suite('ExtHostModelView Validation Tests', () => {
rootComponent.properties.data &&
rootComponent.properties.data[0][0] === testData;
})), Times.once());
declarativeTable.dispose();
});
test('initialized with component data converts to id', async () => {
@@ -416,6 +420,8 @@ suite('ExtHostModelView Validation Tests', () => {
rootComponent.properties.data &&
rootComponent.properties.data[0][0] === button.id;
})), Times.once());
declarativeTable.dispose();
});
test('when added to container with component data converts to id', async () => {
@@ -442,6 +448,8 @@ suite('ExtHostModelView Validation Tests', () => {
item.componentShape.properties.data &&
item.componentShape.properties.data[0][0] === button.id;
})), Times.once());
declarativeTable.dispose();
});
});
});