diff --git a/extensions/machine-learning/src/views/externalLanguages/languagesTable.ts b/extensions/machine-learning/src/views/externalLanguages/languagesTable.ts index 1d9469be54..32c29278e5 100644 --- a/extensions/machine-learning/src/views/externalLanguages/languagesTable.ts +++ b/extensions/machine-learning/src/views/externalLanguages/languagesTable.ts @@ -100,7 +100,7 @@ export class LanguagesTable extends LanguageViewBase { let languages: mssql.ExternalLanguage[] | undefined; languages = await this.listLanguages(); - let tableData: any[][] = []; + let tableData: azdata.DeclarativeTableCellValue[][] = []; if (languages) { @@ -113,10 +113,10 @@ export class LanguagesTable extends LanguageViewBase { }); } - this._table.data = tableData; + this._table.dataValues = tableData; } - private createTableRow(language: mssql.ExternalLanguage, content: mssql.ExternalLanguageContent): any[] { + private createTableRow(language: mssql.ExternalLanguage, content: mssql.ExternalLanguageContent): azdata.DeclarativeTableCellValue[] { if (this._modelBuilder) { let dropLanguageButton = this._modelBuilder.button().withProperties({ label: '', @@ -153,7 +153,7 @@ export class LanguagesTable extends LanguageViewBase { newLang: false }); }); - return [language.name, content.platform, language.createdDate, dropLanguageButton, editLanguageButton]; + return [{ value: language.name }, { value: content.platform || '' }, { value: language.createdDate || '' }, { value: dropLanguageButton }, { value: editLanguageButton }]; } return []; diff --git a/src/sql/azdata.proposed.d.ts b/src/sql/azdata.proposed.d.ts index 6796ea6a2e..e4ce5fbbf6 100644 --- a/src/sql/azdata.proposed.d.ts +++ b/src/sql/azdata.proposed.d.ts @@ -419,7 +419,7 @@ declare module 'azdata' { } export interface DeclarativeTableCellValue { - value: string | number | boolean; + value: string | number | boolean | Component; ariaLabel?: string; style?: CssStyles } diff --git a/src/sql/workbench/browser/modelComponents/declarativeTable.component.ts b/src/sql/workbench/browser/modelComponents/declarativeTable.component.ts index 09970b3d9f..9843096060 100644 --- a/src/sql/workbench/browser/modelComponents/declarativeTable.component.ts +++ b/src/sql/workbench/browser/modelComponents/declarativeTable.component.ts @@ -280,7 +280,17 @@ export default class DeclarativeTableComponent extends ContainerBase { for (let i = 0; i < row.length; i++) { if (this.isComponent(i)) { - this.addToContainer(this.getItemDescriptor(row[i].value as string), undefined); + const itemDescriptor = this.getItemDescriptor(row[i].value as string); + if (itemDescriptor) { + this.addToContainer(itemDescriptor, {}); + } else { + // This should ideally never happen but it's possible for a race condition to happen when adding/removing components quickly where + // the child component is unregistered after it is defined because a component is only unregistered when it's destroyed by Angular + // which can take a while and we don't wait on that to happen currently. + // While this happening isn't desirable it typically doesn't have a huge impact since the component will still be displayed properly in + // most cases + this.logService.warn(`Could not find ItemDescriptor for component ${row[i].value} when adding to DeclarativeTable ${this.descriptor.id}`); + } } } }); diff --git a/src/sql/workbench/browser/modelComponents/modelStore.ts b/src/sql/workbench/browser/modelComponents/modelStore.ts index d12686b34b..75a8b062a2 100644 --- a/src/sql/workbench/browser/modelComponents/modelStore.ts +++ b/src/sql/workbench/browser/modelComponents/modelStore.ts @@ -7,6 +7,7 @@ import { Deferred } from 'sql/base/common/promise'; import { entries } from 'sql/base/common/collections'; import { IComponentDescriptor, IModelStore, IComponent } from 'sql/platform/dashboard/browser/interfaces'; import { onUnexpectedError } from 'vs/base/common/errors'; +import { ILogService } from 'vs/platform/log/common/log'; class ComponentDescriptor implements IComponentDescriptor { constructor(public readonly id: string, public readonly type: string) { @@ -22,7 +23,7 @@ export class ModelStore implements IModelStore { private _componentMappings: { [x: string]: IComponent } = {}; private _componentActions: { [x: string]: { initial: ModelStoreAction[], actions: Deferred } } = {}; private _validationCallbacks: ((componentId: string) => Thenable)[] = []; - constructor() { + constructor(private _logService: ILogService) { } public createComponentDescriptor(type: string, id: string): IComponentDescriptor { @@ -36,12 +37,14 @@ export class ModelStore implements IModelStore { } registerComponent(component: IComponent): void { + this._logService.debug(`Registering component ${component.descriptor.id}`); let id = component.descriptor.id; this._componentMappings[id] = component; this.runPendingActions(id, component); } unregisterComponent(component: IComponent): void { + this._logService.debug(`Unregistering component ${component.descriptor.id}`); let id = component.descriptor.id; this._componentMappings[id] = undefined; this._componentActions[id] = undefined; diff --git a/src/sql/workbench/browser/modelComponents/viewBase.ts b/src/sql/workbench/browser/modelComponents/viewBase.ts index 87b702eb25..6b1c7d89ed 100644 --- a/src/sql/workbench/browser/modelComponents/viewBase.ts +++ b/src/sql/workbench/browser/modelComponents/viewBase.ts @@ -31,7 +31,7 @@ export abstract class ViewBase extends AngularDisposable implements IModelView { public readonly onDestroy = this._onDestroy.event; constructor(protected changeRef: ChangeDetectorRef, protected logService: ILogService) { super(); - this.modelStore = new ModelStore(); + this.modelStore = new ModelStore(logService); } // Properties needed by the model view code @@ -113,7 +113,16 @@ export abstract class ViewBase extends AngularDisposable implements IModelView { removeFromContainer(containerId: string, itemConfig: IItemConfig): void { this.logService.debug(`Queueing action to remove component ${itemConfig.componentShape.id} from container ${containerId}`); - let childDescriptor = this.modelStore.getComponentDescriptor(itemConfig.componentShape.id); + const childDescriptor = this.modelStore.getComponentDescriptor(itemConfig.componentShape.id); + if (!childDescriptor) { + // This should ideally never happen but it's possible for a race condition to happen when adding/removing components quickly where + // the child component is unregistered after it is defined because a component is only unregistered when it's destroyed by Angular + // which can take a while and we don't wait on that to happen currently. + // While this happening isn't desirable there isn't much we can do here currently until that's fixed so for now just continue on since + // it doesn't typically seem to have any huge impacts when this does happen (which is generally rare) + this.logService.warn(`Could not find descriptor for child component ${itemConfig.componentShape.id} when removing from container ${containerId}`); + return; + } this.queueAction(containerId, (component) => { if (!component.removeFromContainer) { this.logService.warn(`Container ${containerId} is trying to remove component ${itemConfig.componentShape.id} but does not implement removeFromContainer!`); diff --git a/src/sql/workbench/test/electron-browser/modalComponents/componentBase.test.ts b/src/sql/workbench/test/electron-browser/modalComponents/componentBase.test.ts index 65af6317dc..cba8261118 100644 --- a/src/sql/workbench/test/electron-browser/modalComponents/componentBase.test.ts +++ b/src/sql/workbench/test/electron-browser/modalComponents/componentBase.test.ts @@ -60,7 +60,7 @@ suite('ComponentBase Tests', () => { let modelStore: IModelStore; setup(() => { - modelStore = new ModelStore(); + modelStore = new ModelStore(new NullLogService()); testComponent = new TestComponent(modelStore, 'testComponent'); testComponent2 = new TestComponent(modelStore, 'testComponent2'); testContainer = new TestContainer(modelStore, 'testContainer');