fix(devtools): prevent race condition between getting nested props an…

…d refresh

projects/ng-devtools-backend/src/lib/component-tree.ts

@@ -45,7 +45,7 @@ export const getLatestComponentState = (query: ComponentExplorerViewQuery): Dire
     }
     if (query.propertyQuery.type === PropertyQueryTypes.Specified) {
       result[dir.name] = {
-        props: deeplySerializeSelectedProperties(dir.instance, query.propertyQuery.properties[dir.name]),
+        props: deeplySerializeSelectedProperties(dir.instance, query.propertyQuery.properties[dir.name] || []),
       };
     }
   };

projects/ng-devtools/src/lib/devtools-tabs/directive-explorer/directive-explorer.component.ts

@@ -55,6 +55,8 @@ export class DirectiveExplorerComponent implements OnInit {
 
   private _changeSize = new Subject<Event>();
   private _clickedElement: IndexedNode | null = null;
+  private _requestingNestedProperties = false;
+  private _refreshScheduled = false;
 
   constructor(
     private _appOperations: ApplicationOperations,
@@ -69,7 +71,6 @@ export class DirectiveExplorerComponent implements OnInit {
   }
 
   ngOnInit(): void {
-    // setInterval(() => this.refresh(), 1);
     this.subscribeToBackendEvents();
   }
 
@@ -95,7 +96,19 @@ export class DirectiveExplorerComponent implements OnInit {
       this.forest = view.forest;
       this.currentSelectedElement = this._clickedElement;
       if (view.properties && this.currentSelectedElement) {
-        this._propResolver.setProperties(this.currentSelectedElement, view.properties);
+        this._propResolver.setProperties(
+          this.currentSelectedElement,
+          view.properties,
+          () => {
+            this._requestingNestedProperties = true;
+          },
+          () => {
+            this._requestingNestedProperties = false;
+            if (this._refreshScheduled) {
+              this.refresh();
+            }
+          }
+        );
       }
     });
 
@@ -106,7 +119,7 @@ export class DirectiveExplorerComponent implements OnInit {
       this.highlightIDinTreeFromElement = null;
     });
 
-    // Only one refresh per 50ms.
+    // Only one refresh per 0.5 seconds.
     let buffering = false;
     this._messageBus.on('componentTreeDirty', () => {
       if (buffering) {
@@ -116,13 +129,18 @@ export class DirectiveExplorerComponent implements OnInit {
       setTimeout(() => {
         buffering = false;
         this.refresh();
-      }, 50);
+      }, 500);
     });
     this.refresh();
   }
 
   refresh(): void {
+    if (this._requestingNestedProperties) {
+      this._refreshScheduled = true;
+      return;
+    }
     this._messageBus.emit('getLatestComponentExplorerView', [this._constructViewQuery()]);
+    this._refreshScheduled = false;
   }
 
   viewSource(): void {
@@ -162,7 +180,7 @@ export class DirectiveExplorerComponent implements OnInit {
     }
     return {
       type: PropertyQueryTypes.Specified,
-      properties: this._propResolver.getExpandedProperties(),
+      properties: this._propResolver.getExpandedProperties() || {},
     };
   }
 

projects/ng-devtools/src/lib/devtools-tabs/directive-explorer/directive-explorer.spec.ts

@@ -1,5 +1,5 @@
 import { DirectiveExplorerComponent } from './directive-explorer.component';
-import { ComponentExplorerViewQuery } from 'protocol';
+import { ComponentExplorerViewQuery, PropertyQueryTypes } from 'protocol';
 import { IndexedNode } from './directive-forest/index-forest';
 import SpyObj = jasmine.SpyObj;
 import { ElementPropertyResolver } from './property-resolver/element-property-resolver';
@@ -28,8 +28,7 @@ describe('DirectiveExplorerComponent', () => {
 
   it('subscribe to backend events', () => {
     comp.subscribeToBackendEvents();
-    expect(messageBusMock.on).toHaveBeenCalledTimes(5);
-    expect(messageBusMock.on).toHaveBeenCalledWith('elementDirectivesProperties', jasmine.any(Function));
+    expect(messageBusMock.on).toHaveBeenCalledTimes(4);
     expect(messageBusMock.on).toHaveBeenCalledWith('latestComponentExplorerView', jasmine.any(Function));
     expect(messageBusMock.on).toHaveBeenCalledWith('highlightComponentInTreeFromElement', jasmine.any(Function));
     expect(messageBusMock.on).toHaveBeenCalledWith('removeHighlightFromComponentTree', jasmine.any(Function));
@@ -44,8 +43,7 @@ describe('DirectiveExplorerComponent', () => {
 
     it('should emit getLatestComponentExplorerView event with null view query', () => {
       comp.refresh();
-      const nullViewQuery: ComponentExplorerViewQuery = { selectedElement: null, expandedProperties: null };
-      expect(messageBusMock.emit).toHaveBeenCalledWith('getLatestComponentExplorerView', [nullViewQuery]);
+      expect(messageBusMock.emit).toHaveBeenCalledWith('getLatestComponentExplorerView', [undefined]);
     });
 
     it('should emit getLatestComponentExplorerView event on refresh with view query no properties', () => {
@@ -60,12 +58,7 @@ describe('DirectiveExplorerComponent', () => {
       };
       comp.refresh();
       expect(comp.currentSelectedElement).toBeTruthy();
-      const viewQuery: ComponentExplorerViewQuery = {
-        // tslint:disable-next-line: no-non-null-assertion
-        selectedElement: comp.currentSelectedElement!.position,
-        expandedProperties: {},
-      };
-      expect(messageBusMock.emit).toHaveBeenCalledWith('getLatestComponentExplorerView', [viewQuery]);
+      expect(messageBusMock.emit).toHaveBeenCalledWith('getLatestComponentExplorerView', [undefined]);
     });
   });
 
@@ -77,11 +70,19 @@ describe('DirectiveExplorerComponent', () => {
     });
 
     it('fires node selection events', () => {
-      nodeMock.position = [0];
+      const position = [0];
+      nodeMock.position = position;
       comp.handleNodeSelection(nodeMock);
       expect(messageBusMock.emit).toHaveBeenCalledTimes(2);
-      expect(messageBusMock.emit).toHaveBeenCalledWith('getElementDirectivesProperties', [nodeMock.position]);
       expect(messageBusMock.emit).toHaveBeenCalledWith('setSelectedComponent', [nodeMock.position]);
+      expect(messageBusMock.emit).toHaveBeenCalledWith('getLatestComponentExplorerView', [
+        {
+          selectedElement: position,
+          propertyQuery: {
+            type: PropertyQueryTypes.All,
+          },
+        },
+      ]);
     });
   });
 });

projects/ng-devtools/src/lib/devtools-tabs/directive-explorer/property-resolver/directive-property-resolver.ts

@@ -40,13 +40,17 @@ export class DirectivePropertyResolver {
     this._treeFlattener,
     this._treeControl,
     this._directivePosition,
-    this._messageBus
+    this._messageBus,
+    this._onRequestingNestedProperties,
+    this._onReceivedNestedProperties
   );
 
   constructor(
     private _messageBus: MessageBus<Events>,
     private _props: Properties,
-    private _directivePosition: DirectivePosition
+    private _directivePosition: DirectivePosition,
+    private _onRequestingNestedProperties: () => void,
+    private _onReceivedNestedProperties: () => void
   ) {}
 
   getDirectiveControls(): { dataSource: PropertyDataSource; treeControl: FlatTreeControl<FlatNode> } {

projects/ng-devtools/src/lib/devtools-tabs/directive-explorer/property-resolver/element-property-resolver.spec.ts

@@ -69,31 +69,41 @@ describe('ElementPropertyResolver', () => {
 
   it('should register directives', () => {
     const resolver = new ElementPropertyResolver(messageBusMock);
-    resolver.setProperties(mockIndexedNode, {
-      FooCmp: {
-        props: {},
-      },
-      BarDir: {
-        props: {},
-      },
-      BazDir: {
-        props: {},
+    resolver.setProperties(
+      mockIndexedNode,
+      {
+        FooCmp: {
+          props: {},
+        },
+        BarDir: {
+          props: {},
+        },
+        BazDir: {
+          props: {},
+        },
       },
-    });
+      () => {},
+      () => {}
+    );
     expect(resolver.getDirectiveController('FooCmp')).not.toBeFalsy();
     expect(resolver.getDirectiveController('BarDir')).not.toBeFalsy();
     expect(resolver.getDirectiveController('BazDir')).not.toBeFalsy();
   });
 
   it('should provide nested props', () => {
     const resolver = new ElementPropertyResolver(messageBusMock);
-    resolver.setProperties(mockIndexedNode, {
-      FooCmp: fooNestedProperties,
-      BarDir: barNestedProps,
-      BazDir: {
-        props: {},
+    resolver.setProperties(
+      mockIndexedNode,
+      {
+        FooCmp: fooNestedProperties,
+        BarDir: barNestedProps,
+        BazDir: {
+          props: {},
+        },
       },
-    });
+      () => {},
+      () => {}
+    );
     const fooController = resolver.getDirectiveController('FooCmp');
     expect(fooController).toBeTruthy();
     // tslint:disable-next-line: no-non-null-assertion

projects/ng-devtools/src/lib/devtools-tabs/directive-explorer/property-resolver/element-property-resolver.ts

@@ -32,7 +32,12 @@ export class ElementPropertyResolver {
     this._directivePropertiesController = new Map();
   }
 
-  setProperties(indexedNode: IndexedNode, data: DirectivesProperties): void {
+  setProperties(
+    indexedNode: IndexedNode,
+    data: DirectivesProperties,
+    onRequestProps: () => void,
+    onReceivedProps: () => void
+  ): void {
     // To prevent memory leaks when a directive no longer exists on an element
     const currentProps = [...this._directivePropertiesController.keys()];
     const incomingProps = new Set(Object.keys(data));
@@ -57,7 +62,7 @@ export class ElementPropertyResolver {
       }
       this._directivePropertiesController.set(
         key,
-        new DirectivePropertyResolver(this._messageBus, data[key], position)
+        new DirectivePropertyResolver(this._messageBus, data[key], position, onRequestProps, onReceivedProps)
       );
     });
   }

projects/ng-devtools/src/lib/devtools-tabs/directive-explorer/property-resolver/property-data-source.ts

@@ -9,7 +9,7 @@ import { diff } from '../../diffing';
 import { FlatNode, Property } from './element-property-resolver';
 
 const trackBy = (_: number, item: FlatNode) => {
-  return `#${item.prop.name}#${item.level}#${item.prop.descriptor.value}`;
+  return `#${item.prop.name}#${item.level}`;
 };
 
 export class PropertyDataSource extends DataSource<FlatNode> {
@@ -23,7 +23,9 @@ export class PropertyDataSource extends DataSource<FlatNode> {
     private _treeFlattener: MatTreeFlattener<Property, FlatNode>,
     private _treeControl: FlatTreeControl<FlatNode>,
     private _entityPosition: DirectivePosition,
-    private _messageBus: MessageBus<Events>
+    private _messageBus: MessageBus<Events>,
+    private _onRequestingNestedProperties: () => void,
+    private _onReceivedNestedProperties: () => void
   ) {
     super();
     this._data.next(this._treeFlattener.flattenNodes(this._arrayify(props)));
@@ -93,6 +95,7 @@ export class PropertyDataSource extends DataSource<FlatNode> {
     }
     parentPath = parentPath.reverse();
 
+    this._onRequestingNestedProperties();
     this._messageBus.emit('getNestedProperties', [this._entityPosition, parentPath]);
 
     this._messageBus.once('nestedProperties', (position: DirectivePosition, data: Properties, path: string[]) => {
@@ -103,6 +106,7 @@ export class PropertyDataSource extends DataSource<FlatNode> {
       flatNodes.forEach(f => (f.level += node.level + 1));
       this.data.splice(index + 1, 0, ...flatNodes);
       this._data.next(this.data);
+      this._onReceivedNestedProperties();
     });
   }