Improve frontend stability (#1482)

This PR improves the stability of the frontend by making two changes: 1. After adding the lock related logic, we have an increasing number of error `ExpressionChangedAfterItHasBeenCheckedError`. When this error occurs, some components on the frontend might become freezing or unresponsive. ![image](https://user-images.githubusercontent.com/12578068/158313495-9afe1ad2-fe22-4198-a6a0-f33cfd3d0384.png) This is due to not running angular change detection after the lock status is changed. We have manually added change detection check whenever lock status is changed. This is also one of Angular's recommended ways to fix it. 2. After adding the async rendering functionaility, we found another JointJS related bug. This error is more likely to happen when switching workflows in version control. When this error happens, JointJS cannot render the operator links. ![image](https://user-images.githubusercontent.com/12578068/158313882-7e9d93ed-d7aa-4a78-8daf-73917f5f4d47.png) This is due to an internal bug in JointJS, see clientIO/joint#1320 . We have adopted a recommended workaround to fix this issue. Thanks @MysteriousChallenger for identifying and solving this issue.
Texera · Mar 17, 2022 · ea2f7be · ea2f7be
1 parent c103a3c
commit ea2f7be
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 20 deletions.
diff --git a/core/new-gui/src/app/dashboard/service/workflow-version/workflow-version.service.ts b/core/new-gui/src/app/dashboard/service/workflow-version/workflow-version.service.ts
@@ -48,9 +48,7 @@ export class WorkflowVersionService {
     // disable the undoredo service because reloading the workflow is considered an action
     this.undoRedoService.disableWorkFlowModification();
     // reload the read only workflow version on the paper
-    // temporarily set JointJS asyncRendering to false to avoid errors,
-    // TODO: fix the error and set asyncRendering to true to improve performance
-    this.workflowActionService.reloadWorkflow(workflow, false);
+    this.workflowActionService.reloadWorkflow(workflow);
     this.setDisplayParticularVersion(true);
     // disable modifications because it is read only
     this.workflowActionService.disableWorkflowModification();
@@ -69,13 +67,12 @@ export class WorkflowVersionService {
   }
 
   public closeParticularVersionDisplay() {
-    console.log("close particular version display called");
     // should enable modifications first to be able to make action of reloading old version on paper
     this.workflowActionService.enableWorkflowModification();
     // but still disable redo and undo service to not capture swapping the workflows, because enabling modifictions automatically enables undo and redo
     this.undoRedoService.disableWorkFlowModification();
     // reload the old workflow don't persist anything
-    this.workflowActionService.reloadWorkflow(this.workflowActionService.getTempWorkflow(), false);
+    this.workflowActionService.reloadWorkflow(this.workflowActionService.getTempWorkflow());
     // clear the temp workflow
     this.workflowActionService.resetTempWorkflow();
     // after reloading the workflow, we can enable the undoredo service

diff --git a/core/new-gui/src/app/workspace/component/navigation/navigation.component.ts b/core/new-gui/src/app/workspace/component/navigation/navigation.component.ts
@@ -1,13 +1,12 @@
 import { DatePipe, Location } from "@angular/common";
-import { Component, ElementRef, Input, ViewChild } from "@angular/core";
+import { ChangeDetectorRef, Component, ElementRef, Input, OnInit, ViewChild } from "@angular/core";
 import { environment } from "../../../../environments/environment";
 import { UserService } from "../../../common/service/user/user.service";
 import { WorkflowPersistService } from "../../../common/service/workflow-persist/workflow-persist.service";
 import { Workflow } from "../../../common/type/workflow";
 import { ExecuteWorkflowService } from "../../service/execute-workflow/execute-workflow.service";
 import { UndoRedoService } from "../../service/undo-redo/undo-redo.service";
 import { ValidationWorkflowService } from "../../service/validation/validation-workflow.service";
-import { WorkflowCacheService } from "../../service/workflow-cache/workflow-cache.service";
 import { JointGraphWrapper } from "../../service/workflow-graph/model/joint-graph-wrapper";
 import { WorkflowActionService } from "../../service/workflow-graph/model/workflow-action.service";
 import { ExecutionState } from "../../types/execute-workflow.interface";
@@ -42,7 +41,7 @@ import { WorkflowCollabService } from "../../service/workflow-collab/workflow-co
   templateUrl: "./navigation.component.html",
   styleUrls: ["./navigation.component.scss"],
 })
-export class NavigationComponent {
+export class NavigationComponent implements OnInit {
   public executionState: ExecutionState; // set this to true when the workflow is started
   public ExecutionState = ExecutionState; // make Angular HTML access enum definition
   public isWorkflowValid: boolean = true; // this will check whether the workflow error or not
@@ -87,11 +86,11 @@ export class NavigationComponent {
     public workflowPersistService: WorkflowPersistService,
     public workflowVersionService: WorkflowVersionService,
     public userService: UserService,
-    private workflowCacheService: WorkflowCacheService,
     private datePipe: DatePipe,
     public workflowResultExportService: WorkflowResultExportService,
     public workflowCollabService: WorkflowCollabService,
-    public workflowUtilService: WorkflowUtilService
+    public workflowUtilService: WorkflowUtilService,
+    public changeDetectionRef: ChangeDetectorRef
   ) {
     this.executionState = executeWorkflowService.getExecutionState().state;
     // return the run button after the execution is finished, either
@@ -102,8 +101,10 @@ export class NavigationComponent {
     this.runDisable = initBehavior.disable;
     this.onClickRunHandler = initBehavior.onClick;
     // this.currentWorkflowName = this.workflowCacheService.getCachedWorkflow();
+  }
 
-    executeWorkflowService
+  public ngOnInit(): void {
+    this.executeWorkflowService
       .getExecutionStateStream()
       .pipe(untilDestroyed(this))
       .subscribe(event => {
@@ -112,7 +113,7 @@ export class NavigationComponent {
       });
 
     // set the map of operatorStatusMap
-    validationWorkflowService
+    this.validationWorkflowService
       .getWorkflowValidationErrorStream()
       .pipe(untilDestroyed(this))
       .subscribe(value => {
@@ -536,6 +537,7 @@ export class NavigationComponent {
       .pipe(untilDestroyed(this))
       .subscribe((lockGranted: boolean) => {
         this.lockGranted = lockGranted;
+        this.changeDetectionRef.detectChanges();
       });
   }
 

diff --git a/...gui/src/app/workspace/component/operator-panel/operator-label/operator-label.component.ts b/...gui/src/app/workspace/component/operator-panel/operator-label/operator-label.component.ts
@@ -1,6 +1,6 @@
 import { DragDropService } from "../../../service/drag-drop/drag-drop.service";
 import { WorkflowActionService } from "../../../service/workflow-graph/model/workflow-action.service";
-import { AfterViewInit, Component, Input, OnInit } from "@angular/core";
+import { AfterContentInit, AfterViewInit, ChangeDetectorRef, Component, Input, OnInit } from "@angular/core";
 
 import { OperatorSchema } from "../../../types/operator-schema.interface";
 import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy";
@@ -16,7 +16,7 @@ import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy";
   templateUrl: "./operator-label.component.html",
   styleUrls: ["./operator-label.component.scss"],
 })
-export class OperatorLabelComponent implements OnInit, AfterViewInit {
+export class OperatorLabelComponent implements OnInit, AfterViewInit, AfterContentInit {
   public static operatorLabelPrefix = "texera-operator-label-";
   public static operatorLabelSearchBoxPrefix = "texera-operator-label-search-result-";
 
@@ -33,9 +33,15 @@ export class OperatorLabelComponent implements OnInit, AfterViewInit {
   // is mouse down over this label
   private isMouseDown = false;
 
-  constructor(private dragDropService: DragDropService, private workflowActionService: WorkflowActionService) {}
+  constructor(
+    private dragDropService: DragDropService,
+    private workflowActionService: WorkflowActionService,
+    private changeDetectorRef: ChangeDetectorRef
+  ) {}
 
-  ngOnInit() {
+  ngOnInit() {}
+
+  ngAfterContentInit(): void {
     this.handleWorkFlowModificationEnabled();
     if (!this.operator) {
       throw new Error("operator label component: operator is not specified");
@@ -90,6 +96,7 @@ export class OperatorLabelComponent implements OnInit, AfterViewInit {
       .pipe(untilDestroyed(this))
       .subscribe(canModify => {
         this.draggable = canModify;
+        this.changeDetectorRef.detectChanges();
       });
   }
 

diff --git a/...nt/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts b/...nt/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts
@@ -1,4 +1,4 @@
-import { Component, Input, OnChanges, OnDestroy, OnInit, SimpleChanges } from "@angular/core";
+import { ChangeDetectorRef, Component, Input, OnChanges, OnDestroy, OnInit, SimpleChanges } from "@angular/core";
 import { ExecuteWorkflowService } from "../../../service/execute-workflow/execute-workflow.service";
 import { Subject } from "rxjs";
 import { FormGroup } from "@angular/forms";
@@ -101,7 +101,8 @@ export class OperatorPropertyEditFrameComponent implements OnInit, OnChanges, On
     private dynamicSchemaService: DynamicSchemaService,
     private schemaPropagationService: SchemaPropagationService,
     private notificationService: NotificationService,
-    private workflowCollabService: WorkflowCollabService
+    private workflowCollabService: WorkflowCollabService,
+    private changeDetectorRef: ChangeDetectorRef
   ) {}
 
   ngOnChanges(changes: SimpleChanges): void {
@@ -296,6 +297,7 @@ export class OperatorPropertyEditFrameComponent implements OnInit, OnChanges, On
         if (this.currentOperatorId) {
           const interactive = this.evaluateInteractivity();
           this.setInteractivity(interactive);
+          this.changeDetectorRef.detectChanges();
         }
       });
   }
@@ -316,6 +318,7 @@ export class OperatorPropertyEditFrameComponent implements OnInit, OnChanges, On
       .pipe(untilDestroyed(this))
       .subscribe((lockGranted: boolean) => {
         this.lockGranted = lockGranted;
+        this.changeDetectorRef.detectChanges();
       });
   }
 

diff --git a/core/new-gui/src/app/workspace/component/workflow-editor/workflow-editor.component.ts b/core/new-gui/src/app/workspace/component/workflow-editor/workflow-editor.component.ts
@@ -1,4 +1,4 @@
-import { AfterViewInit, Component, ElementRef } from "@angular/core";
+import { AfterViewInit, ChangeDetectorRef, Component, ElementRef } from "@angular/core";
 import * as joint from "jointjs";
 // if jQuery needs to be used: 1) use jQuery instead of `$`, and
 // 2) always add this import statement even if TypeScript doesn't show an error https://github.com/Microsoft/TypeScript/issues/22016
@@ -107,7 +107,8 @@ export class WorkflowEditorComponent implements AfterViewInit {
     private workflowStatusService: WorkflowStatusService,
     private workflowUtilService: WorkflowUtilService,
     private executeWorkflowService: ExecuteWorkflowService,
-    private nzModalService: NzModalService
+    private nzModalService: NzModalService,
+    private changeDetectorRef: ChangeDetectorRef
   ) {}
 
   public getJointPaper(): joint.dia.Paper {
@@ -181,6 +182,7 @@ export class WorkflowEditorComponent implements AfterViewInit {
           this.interactive = false;
           this.getJointPaper().setInteractivity(disableInteractiveOption);
         }
+        this.changeDetectorRef.detectChanges();
       });
   }
 
@@ -953,6 +955,9 @@ export class WorkflowEditorComponent implements AfterViewInit {
       },
       // set grid size
       gridSize: 2,
+      // use approximate z-index sorting, this is a workaround of a bug in async rendering mode
+      // see https://github.com/clientIO/joint/issues/1320
+      sorting: joint.dia.Paper.sorting.APPROX,
     };
 
     return jointPaperOptions;

diff --git a/core/new-gui/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts b/core/new-gui/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts
@@ -131,12 +131,18 @@ export class WorkflowActionService {
 
   // workflow modification lock interface (allows or prevents commands that would modify the workflow graph)
   public enableWorkflowModification() {
+    if (this.workflowModificationEnabled) {
+      return;
+    }
     this.workflowModificationEnabled = true;
     this.enableModificationStream.next(true);
     this.undoRedoService.enableWorkFlowModification();
   }
 
   public disableWorkflowModification() {
+    if (!this.workflowModificationEnabled) {
+      return;
+    }
     this.workflowModificationEnabled = false;
     this.enableModificationStream.next(false);
     this.undoRedoService.disableWorkFlowModification();