refactor(cf): use Observable to set state to prevent memory leaks (sp…

…innaker#6832)
christopherthielen · Apr 10, 2019 · 7338f7d · 7338f7d
1 parent ca3fcef
commit 7338f7d
Show file tree

Hide file tree

Showing 18 changed files with 329 additions and 166 deletions.
diff --git a/app/scripts/modules/cloudfoundry/src/loadBalancer/configure/loadBalancerDetails.tsx b/app/scripts/modules/cloudfoundry/src/loadBalancer/configure/loadBalancerDetails.tsx
@@ -1,5 +1,7 @@
 import * as React from 'react';
 
+import { Observable, Subject } from 'rxjs';
+
 import Select, { Option } from 'react-select';
 
 import { FormikErrors, FormikProps } from 'formik';
@@ -25,6 +27,7 @@ export interface ILoadBalancerDetailsState {
 
 export class LoadBalancerDetails extends React.Component<ILoadBalancerDetailsProps, ILoadBalancerDetailsState>
   implements IWizardPageComponent<ICloudFoundryLoadBalancerUpsertCommand> {
+  private destroy$ = new Subject();
   public state: ILoadBalancerDetailsState = {
     accounts: undefined,
     availabilityZones: [],
@@ -60,11 +63,17 @@ export class LoadBalancerDetails extends React.Component<ILoadBalancerDetailsPro
     this.loadAccounts();
   }
 
+  public componentWillUnmount(): void {
+    this.destroy$.next();
+  }
+
   private loadAccounts(): void {
-    AccountService.listAccounts('cloudfoundry').then(accounts => {
-      this.setState({ accounts });
-      this.loadDomainsAndRegions();
-    });
+    Observable.fromPromise(AccountService.listAccounts('cloudfoundry'))
+      .takeUntil(this.destroy$)
+      .subscribe(accounts => {
+        this.setState({ accounts });
+        this.loadDomainsAndRegions();
+      });
   }
 
   private accountUpdated = (option: Option<string>): void => {
@@ -76,12 +85,12 @@ export class LoadBalancerDetails extends React.Component<ILoadBalancerDetailsPro
   private loadDomainsAndRegions(): void {
     const account = this.props.formik.values.credentials;
     if (account) {
-      AccountService.getAccountDetails(account).then((accountDetails: ICloudFoundryAccount) => {
-        this.setState({ domains: accountDetails.domains });
-      });
-      AccountService.getRegionsForAccount(account).then(regions => {
-        this.setState({ regions });
-      });
+      Observable.fromPromise(AccountService.getAccountDetails(account))
+        .takeUntil(this.destroy$)
+        .subscribe((accountDetails: ICloudFoundryAccount) => this.setState({ domains: accountDetails.domains }));
+      Observable.fromPromise(AccountService.getRegionsForAccount(account))
+        .takeUntil(this.destroy$)
+        .subscribe(regions => this.setState({ regions }));
     }
   }
 
@@ -90,14 +99,16 @@ export class LoadBalancerDetails extends React.Component<ILoadBalancerDetailsPro
     this.props.formik.setFieldValue('region', region);
     if (region) {
       const { credentials } = this.props.formik.values;
-      AccountService.getAccountDetails(credentials).then((accountDetails: ICloudFoundryAccount) => {
-        const { domains } = accountDetails;
-        this.setState({
-          domains: domains.filter(
-            domain => domain.organization === undefined || region.match('^' + domain.organization.name),
-          ),
+      Observable.fromPromise(AccountService.getAccountDetails(credentials))
+        .takeUntil(this.destroy$)
+        .subscribe((accountDetails: ICloudFoundryAccount) => {
+          const { domains } = accountDetails;
+          this.setState({
+            domains: domains.filter(
+              domain => domain.organization === undefined || region.match('^' + domain.organization.name),
+            ),
+          });
         });
-      });
     }
   };
 

diff --git a/...dfoundry/src/pipeline/stages/createServiceKey/CloudfoundryCreateServiceKeyStageConfig.tsx b/...dfoundry/src/pipeline/stages/createServiceKey/CloudfoundryCreateServiceKeyStageConfig.tsx
@@ -1,5 +1,7 @@
 import * as React from 'react';
 
+import { Observable, Subject } from 'rxjs';
+
 import { Option } from 'react-select';
 
 import {
@@ -21,6 +23,8 @@ export class CloudfoundryCreateServiceKeyStageConfig extends React.Component<
   IStageConfigProps,
   ICloudfoundryCreateServiceKeyStageConfigState
 > {
+  private destroy$ = new Subject();
+
   constructor(props: IStageConfigProps) {
     super(props);
     props.stage.cloudProvider = 'cloudfoundry';
@@ -30,22 +34,28 @@ export class CloudfoundryCreateServiceKeyStageConfig extends React.Component<
     };
   }
 
-  public componentDidMount = () => {
-    AccountService.listAccounts('cloudfoundry').then((rawAccounts: IAccount[]) => {
-      this.setState({ accounts: rawAccounts.map(it => it.name) });
-    });
+  public componentDidMount(): void {
+    Observable.fromPromise(AccountService.listAccounts('cloudfoundry'))
+      .takeUntil(this.destroy$)
+      .subscribe((rawAccounts: IAccount[]) => this.setState({ accounts: rawAccounts.map(it => it.name) }));
     if (this.props.stage.credentials) {
       this.clearAndReloadRegions();
     }
-  };
+  }
+
+  public componentWillUnmount(): void {
+    this.destroy$.next();
+  }
 
   private clearAndReloadRegions = () => {
     this.setState({ regions: [] });
-    AccountService.getRegionsForAccount(this.props.stage.credentials).then((regionList: IRegion[]) => {
-      const regions = regionList.map(r => r.name);
-      regions.sort((a, b) => a.localeCompare(b));
-      this.setState({ regions });
-    });
+    Observable.fromPromise(AccountService.getRegionsForAccount(this.props.stage.credentials))
+      .takeUntil(this.destroy$)
+      .subscribe((regionList: IRegion[]) => {
+        const regions = regionList.map(r => r.name);
+        regions.sort((a, b) => a.localeCompare(b));
+        this.setState({ regions });
+      });
   };
 
   private serviceInstanceNameUpdated = (event: React.ChangeEvent<HTMLInputElement>) => {

diff --git a/...s/cloudfoundry/src/pipeline/stages/deployService/CloudfoundryDeployServiceStageConfig.tsx b/...s/cloudfoundry/src/pipeline/stages/deployService/CloudfoundryDeployServiceStageConfig.tsx
@@ -1,6 +1,9 @@
 import * as React from 'react';
+
 import { Option } from 'react-select';
 
+import { Observable, Subject } from 'rxjs';
+
 import {
   AccountService,
   IAccount,
@@ -38,10 +41,7 @@ export class CloudfoundryDeployServiceStageConfig extends React.Component<
       updatable: true,
     },
   };
-
-  // Hack necessary probably because of stage.module.js manually unmounting the component.
-  // There is no apparent way to cancel IPromises generated by AccountService.
-  private mounted = true;
+  private destroy$ = new Subject();
 
   constructor(props: IStageConfigProps) {
     super(props);
@@ -62,24 +62,23 @@ export class CloudfoundryDeployServiceStageConfig extends React.Component<
     this.reloadRegions();
   };
 
-  public componentWillUnmount(): void {
-    this.mounted = false;
+  public componentDidMount(): void {
+    Observable.fromPromise(AccountService.listAccounts('cloudfoundry'))
+      .takeUntil(this.destroy$)
+      .subscribe(accounts => this.setState({ accounts }));
+    this.reloadRegions();
   }
 
-  public componentDidMount = (): void => {
-    AccountService.listAccounts('cloudfoundry').then(accounts => {
-      if (this.mounted) {
-        this.setState({ accounts: accounts });
-      }
-    });
-    this.reloadRegions();
-  };
+  public componentWillUnmount(): void {
+    this.destroy$.next();
+  }
 
   private reloadRegions = () => {
-    if (this.props.stage.credentials) {
-      if (this.mounted) {
-        AccountService.getRegionsForAccount(this.props.stage.credentials).then(regions => this.setState({ regions }));
-      }
+    const { credentials } = this.props.stage;
+    if (credentials) {
+      Observable.fromPromise(AccountService.getRegionsForAccount(credentials))
+        .takeUntil(this.destroy$)
+        .subscribe(regions => this.setState({ regions }));
     }
   };
 

diff --git a/...dules/cloudfoundry/src/pipeline/stages/deployService/CreateServiceInstanceDirectInput.tsx b/...dules/cloudfoundry/src/pipeline/stages/deployService/CreateServiceInstanceDirectInput.tsx
@@ -1,6 +1,7 @@
 import * as React from 'react';
 
 import { Option } from 'react-select';
+import { Observable, Subject } from 'rxjs';
 
 import {
   IService,
@@ -30,6 +31,7 @@ export class CreateServiceInstanceDirectInput extends React.Component<
   ICreateServiceInstanceDirectInputProps,
   ICreateServiceInstanceDirectInputState
 > {
+  private destroy$ = new Subject();
   constructor(props: ICreateServiceInstanceDirectInputProps) {
     super(props);
     this.state = { serviceNamesAndPlans: [] };
@@ -42,15 +44,19 @@ export class CreateServiceInstanceDirectInput extends React.Component<
     }
   }
 
-  public componentDidMount = () => {
+  public componentDidMount(): void {
     this.loadServices(this.props.credentials, this.props.region);
-  };
+  }
+
+  public componentWillUnmount(): void {
+    this.destroy$.next();
+  }
 
   private loadServices(credentials: string, region: string) {
     if (credentials && region) {
-      ServicesReader.getServices(credentials, region).then(serviceNamesAndPlans => {
-        this.setState({ serviceNamesAndPlans });
-      });
+      Observable.fromPromise(ServicesReader.getServices(credentials, region))
+        .takeUntil(this.destroy$)
+        .subscribe(serviceNamesAndPlans => this.setState({ serviceNamesAndPlans }));
     }
   }
 

diff --git a/...modules/cloudfoundry/src/pipeline/stages/destroyAsg/CloudfoundryDestroyAsgStageConfig.tsx b/...modules/cloudfoundry/src/pipeline/stages/destroyAsg/CloudfoundryDestroyAsgStageConfig.tsx
@@ -1,4 +1,7 @@
 import * as React from 'react';
+
+import { Observable, Subject } from 'rxjs';
+
 import {
   AccountService,
   Application,
@@ -33,6 +36,7 @@ export class CloudfoundryDestroyAsgStageConfig extends React.Component<
   ICloudfoundryDestroyAsgStageProps,
   ICloudfoundryDestroyAsgStageConfigState
 > {
+  private destroy$ = new Subject();
   constructor(props: ICloudfoundryDestroyAsgStageProps) {
     super(props);
     props.stage.cloudProvider = 'cloudfoundry';
@@ -49,20 +53,26 @@ export class CloudfoundryDestroyAsgStageConfig extends React.Component<
     };
   }
 
-  public componentDidMount = (): void => {
-    AccountService.listAccounts('cloudfoundry').then(accounts => {
-      this.setState({ accounts });
-      this.accountUpdated();
-    });
+  public componentDidMount(): void {
+    Observable.fromPromise(AccountService.listAccounts('cloudfoundry'))
+      .takeUntil(this.destroy$)
+      .subscribe(accounts => {
+        this.setState({ accounts });
+        this.accountUpdated();
+      });
     this.props.stageFieldUpdated();
-  };
+  }
+
+  public componentWillUnmount(): void {
+    this.destroy$.next();
+  }
 
   private accountUpdated = (): void => {
     const { credentials } = this.props.stage;
     if (credentials) {
-      AccountService.getRegionsForAccount(credentials).then(regions => {
-        this.setState({ regions });
-      });
+      Observable.fromPromise(AccountService.getRegionsForAccount(credentials))
+        .takeUntil(this.destroy$)
+        .subscribe(regions => this.setState({ regions }));
     }
   };
 

diff --git a/...cloudfoundry/src/pipeline/stages/destroyService/CloudfoundryDestroyServiceStageConfig.tsx b/...cloudfoundry/src/pipeline/stages/destroyService/CloudfoundryDestroyServiceStageConfig.tsx
@@ -1,6 +1,7 @@
 import * as React from 'react';
 
 import Select, { Option } from 'react-select';
+import { Observable, Subject } from 'rxjs';
 
 import { AccountService, IAccount, IRegion, IStageConfigProps, StageConfigField } from '@spinnaker/core';
 
@@ -13,6 +14,7 @@ export class CloudfoundryDestroyServiceStageConfig extends React.Component<
   IStageConfigProps,
   ICloudfoundryDestroyServiceStageConfigState
 > {
+  private destroy$ = new Subject();
   constructor(props: IStageConfigProps) {
     super(props);
     this.props.updateStageField({ cloudProvider: 'cloudfoundry' });
@@ -22,20 +24,29 @@ export class CloudfoundryDestroyServiceStageConfig extends React.Component<
     };
   }
 
-  public componentDidMount = () => {
-    AccountService.listAccounts('cloudfoundry').then((accounts: IAccount[]) => {
-      this.setState({ accounts });
-      if (this.props.stage.credentials) {
-        this.clearAndReloadRegions();
-      }
-    });
-  };
+  public componentDidMount(): void {
+    Observable.fromPromise(AccountService.listAccounts('cloudfoundry'))
+      .takeUntil(this.destroy$)
+      .subscribe(accounts => {
+        this.setState({ accounts });
+        if (this.props.stage.credentials) {
+          this.clearAndReloadRegions();
+        }
+      });
+  }
+
+  public componentWillUnmount(): void {
+    this.destroy$.next();
+  }
 
   private clearAndReloadRegions = () => {
     this.setState({ regions: [] });
-    AccountService.getRegionsForAccount(this.props.stage.credentials).then((regions: IRegion[]) =>
-      this.setState({ regions }),
-    );
+    const { credentials } = this.props.stage;
+    if (credentials) {
+      Observable.fromPromise(AccountService.getRegionsForAccount(credentials))
+        .takeUntil(this.destroy$)
+        .subscribe(regions => this.setState({ regions }));
+    }
   };
 
   private accountUpdated = (option: Option<string>) => {

diff --git a/...modules/cloudfoundry/src/pipeline/stages/disableAsg/CloudfoundryDisableAsgStageConfig.tsx b/...modules/cloudfoundry/src/pipeline/stages/disableAsg/CloudfoundryDisableAsgStageConfig.tsx
@@ -1,4 +1,7 @@
 import * as React from 'react';
+
+import { Observable, Subject } from 'rxjs';
+
 import {
   AccountService,
   Application,
@@ -32,6 +35,8 @@ export class CloudfoundryDisableAsgStageConfig extends React.Component<
   ICloudfoundryDisableAsgStageProps,
   ICloudfoundryDisableAsgStageConfigState
 > {
+  private destroy$ = new Subject();
+
   constructor(props: ICloudfoundryDisableAsgStageProps) {
     super(props);
     props.stage.cloudProvider = 'cloudfoundry';
@@ -47,20 +52,26 @@ export class CloudfoundryDisableAsgStageConfig extends React.Component<
     };
   }
 
-  public componentDidMount = (): void => {
-    AccountService.listAccounts('cloudfoundry').then(accounts => {
-      this.setState({ accounts: accounts });
-      this.accountUpdated();
-    });
+  public componentDidMount(): void {
+    Observable.fromPromise(AccountService.listAccounts('cloudfoundry'))
+      .takeUntil(this.destroy$)
+      .subscribe(accounts => {
+        this.setState({ accounts });
+        this.accountUpdated();
+      });
     this.props.stageFieldUpdated();
-  };
+  }
+
+  public componentWillUnmount(): void {
+    this.destroy$.next();
+  }
 
   private accountUpdated = (): void => {
     const { credentials } = this.props.stage;
     if (credentials) {
-      AccountService.getRegionsForAccount(credentials).then(regions => {
-        this.setState({ regions: regions });
-      });
+      Observable.fromPromise(AccountService.getRegionsForAccount(credentials))
+        .takeUntil(this.destroy$)
+        .subscribe(regions => this.setState({ regions }));
     }
   };