Fix declarative table display issues with ML ext (#13529)

* Fix declarative table display issues with ML ext

* Fix test

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 [];

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
 	}

src/sql/workbench/browser/modelComponents/declarativeTable.component.ts

@@ -280,7 +280,17 @@ export default class DeclarativeTableComponent extends ContainerBase<any, azdata
 			this.data?.forEach(row => {
 				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}`);
+						}
 					}
 				}
 			});

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<any>[], actions: Deferred<IComponent> } } = {};
 	private _validationCallbacks: ((componentId: string) => Thenable<boolean>)[] = [];
-	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;

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!`);

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');