Browse Source

fix(reflux): add store teardown and make sure .init doesnt leak memory (#32958)

* fix(guidestore): cleanup listeners

* fix(modalstore): cleanup listeners

* fix(reflux): add a wrapper to store

* fix(reflux): use safe store with all stores that implement init

* fix(hookstore): fix tests

* fix(reflux): test overriding instance method

* fix(stores): add teardown to almost all stores

* fix(stores): attempt to isolate state in groupingStore test

* fix(stores): fix makeSafeReflux store import

* fix(stores): remove internals from alertstore

* fix(stores): remove internals from debug store and fix type cast

* fix(tests): fix projectfilters flake

* fix(safeRefluxStore): remove interface export

* fix(safeRefluxStore): get in there before init is called

* fix(safeRefluxStore): cast to SafeRefluxStore and add doc comment

* Revert "fix(safeRefluxStore): cast to SafeRefluxStore and add doc comment"

This reverts commit d472cda9af1af5572aabe909cb5bf14f36b61ce7.

* fix(safeRefluxStore): add jsdoc comment

* fix(safeRefluxStore): allow extending Reflux.Store

* feat(reflux): cast to SafeRefluxStore so we have proper types

* feat(test): avoid using regexp
Jonas 2 years ago
parent
commit
7b93cd7e4c

+ 4 - 2
static/app/stores/committerStore.tsx

@@ -2,6 +2,7 @@ import Reflux from 'reflux';
 
 import CommitterActions from 'sentry/actions/committerActions';
 import {Committer} from 'sentry/types';
+import {makeSafeRefluxStore, SafeRefluxStore} from 'sentry/utils/makeSafeRefluxStore';
 
 type State = {
   // Use `getCommitterStoreKey` to generate key
@@ -104,7 +105,8 @@ export function getCommitterStoreKey(
   return `${orgSlug} ${projectSlug} ${eventId}`;
 }
 
-const CommitterStore = Reflux.createStore(storeConfig) as Reflux.Store &
-  CommitterStoreInterface;
+const CommitterStore = Reflux.createStore(
+  makeSafeRefluxStore(storeConfig)
+) as SafeRefluxStore & CommitterStoreInterface;
 
 export default CommitterStore;

+ 4 - 2
static/app/stores/configStore.tsx

@@ -2,6 +2,7 @@ import moment from 'moment-timezone';
 import Reflux from 'reflux';
 
 import {Config} from 'sentry/types';
+import {makeSafeRefluxStore, SafeRefluxStore} from 'sentry/utils/makeSafeRefluxStore';
 
 import {CommonStoreInterface} from './types';
 
@@ -77,7 +78,8 @@ const storeConfig: Reflux.StoreDefinition & Internals & ConfigStoreInterface = {
   },
 };
 
-const ConfigStore = Reflux.createStore(storeConfig) as Reflux.Store &
-  ConfigStoreInterface;
+const ConfigStore = Reflux.createStore(
+  makeSafeRefluxStore(storeConfig)
+) as SafeRefluxStore & ConfigStoreInterface;
 
 export default ConfigStore;

+ 18 - 3
static/app/stores/debugMetaStore.tsx

@@ -1,5 +1,11 @@
 import Reflux from 'reflux';
 
+import {
+  makeSafeRefluxStore,
+  SafeRefluxStore,
+  SafeStoreDefinition,
+} from 'sentry/utils/makeSafeRefluxStore';
+
 const DebugMetaActions = Reflux.createActions(['updateFilter']);
 
 type State = {
@@ -17,12 +23,19 @@ type Internals = {
   filter: string | null;
 };
 
-const storeConfig: Reflux.StoreDefinition & DebugMetaStoreInterface & Internals = {
+const storeConfig: Reflux.StoreDefinition &
+  DebugMetaStoreInterface &
+  Internals &
+  SafeStoreDefinition = {
   filter: null,
+  unsubscribeListeners: [],
 
   init() {
     this.reset();
-    this.listenTo(DebugMetaActions.updateFilter, this.updateFilter);
+
+    this.unsubscribeListeners.push(
+      this.listenTo(DebugMetaActions.updateFilter, this.updateFilter)
+    );
   },
 
   reset() {
@@ -42,7 +55,9 @@ const storeConfig: Reflux.StoreDefinition & DebugMetaStoreInterface & Internals
   },
 };
 
-const DebugMetaStore = Reflux.createStore(storeConfig);
+const DebugMetaStore = Reflux.createStore(
+  makeSafeRefluxStore(storeConfig)
+) as SafeRefluxStore & DebugMetaStoreInterface;
 
 export {DebugMetaActions, DebugMetaStore};
 export default DebugMetaStore;

+ 4 - 1
static/app/stores/eventStore.tsx

@@ -3,6 +3,7 @@ import isArray from 'lodash/isArray';
 import Reflux from 'reflux';
 
 import {Event} from 'sentry/types/event';
+import {makeSafeRefluxStore, SafeRefluxStore} from 'sentry/utils/makeSafeRefluxStore';
 
 type Internals = {
   items: Event[];
@@ -98,6 +99,8 @@ const storeConfig: Reflux.StoreDefinition & Internals & EventStoreInterface = {
   },
 };
 
-const EventStore = Reflux.createStore(storeConfig) as Reflux.Store & EventStoreInterface;
+const EventStore = Reflux.createStore(
+  makeSafeRefluxStore(storeConfig)
+) as SafeRefluxStore & EventStoreInterface;
 
 export default EventStore;

+ 4 - 2
static/app/stores/externalIssueStore.tsx

@@ -1,6 +1,7 @@
 import Reflux from 'reflux';
 
 import {PlatformExternalIssue} from 'sentry/types';
+import {makeSafeRefluxStore, SafeRefluxStore} from 'sentry/utils/makeSafeRefluxStore';
 
 type ExternalIssueStoreInterface = {
   add(issue: PlatformExternalIssue): void;
@@ -38,7 +39,8 @@ const storeConfig: Reflux.StoreDefinition & ExternalIssueStoreInterface = {
   },
 };
 
-const ExternalIssueStore = Reflux.createStore(storeConfig) as Reflux.Store &
-  ExternalIssueStoreInterface;
+const ExternalIssueStore = Reflux.createStore(
+  makeSafeRefluxStore(storeConfig)
+) as SafeRefluxStore & ExternalIssueStoreInterface;
 
 export default ExternalIssueStore;

+ 12 - 3
static/app/stores/formSearchStore.tsx

@@ -2,6 +2,7 @@ import Reflux from 'reflux';
 
 import FormSearchActions from 'sentry/actions/formSearchActions';
 import {FieldObject} from 'sentry/components/forms/type';
+import {makeSafeRefluxStore, SafeStoreDefinition} from 'sentry/utils/makeSafeRefluxStore';
 
 /**
  * Processed form field metadata.
@@ -26,12 +27,18 @@ type Internals = {
 /**
  * Store for "form" searches, but probably will include more
  */
-const storeConfig: Reflux.StoreDefinition & Internals & StoreInterface = {
+const storeConfig: Reflux.StoreDefinition &
+  Internals &
+  StoreInterface &
+  SafeStoreDefinition = {
   searchMap: null,
+  unsubscribeListeners: [],
 
   init() {
     this.reset();
-    this.listenTo(FormSearchActions.loadSearchMap, this.onLoadSearchMap);
+    this.unsubscribeListeners.push(
+      this.listenTo(FormSearchActions.loadSearchMap, this.onLoadSearchMap)
+    );
   },
 
   get() {
@@ -57,6 +64,8 @@ const storeConfig: Reflux.StoreDefinition & Internals & StoreInterface = {
   },
 };
 
-const FormSearchStore = Reflux.createStore(storeConfig) as Reflux.Store & StoreInterface;
+const FormSearchStore = Reflux.createStore(
+  makeSafeRefluxStore(storeConfig)
+) as Reflux.Store & StoreInterface;
 
 export default FormSearchStore;

+ 4 - 1
static/app/stores/groupStore.tsx

@@ -13,6 +13,7 @@ import {
   GroupRelease,
   GroupStats,
 } from 'sentry/types';
+import {makeSafeRefluxStore, SafeRefluxStore} from 'sentry/utils/makeSafeRefluxStore';
 
 function showAlert(msg, type) {
   IndicatorStore.addMessage(msg, type, {duration: 4000});
@@ -511,6 +512,8 @@ const storeConfig: Reflux.StoreDefinition & Internals & GroupStoreInterface = {
   },
 };
 
-const GroupStore = Reflux.createStore(storeConfig) as Reflux.Store & GroupStoreInterface;
+const GroupStore = Reflux.createStore(
+  makeSafeRefluxStore(storeConfig)
+) as SafeRefluxStore & GroupStoreInterface;
 
 export default GroupStore;

+ 4 - 2
static/app/stores/groupingStore.tsx

@@ -11,6 +11,7 @@ import GroupingActions from 'sentry/actions/groupingActions';
 import {Client} from 'sentry/api';
 import {Group, Organization, Project} from 'sentry/types';
 import {Event} from 'sentry/types/event';
+import {makeSafeRefluxStore, SafeRefluxStore} from 'sentry/utils/makeSafeRefluxStore';
 
 // Between 0-100
 const MIN_SCORE = 0.6;
@@ -616,7 +617,8 @@ const storeConfig: Reflux.StoreDefinition & Internals & GroupingStoreInterface =
   },
 };
 
-const GroupingStore = Reflux.createStore(storeConfig) as Reflux.Store &
-  GroupingStoreInterface;
+const GroupingStore = Reflux.createStore(
+  makeSafeRefluxStore(storeConfig)
+) as SafeRefluxStore & GroupingStoreInterface;
 
 export default GroupingStore;

+ 42 - 11
static/app/stores/guideStore.tsx

@@ -8,6 +8,12 @@ import getGuidesContent from 'sentry/components/assistant/getGuidesContent';
 import {Guide, GuidesContent, GuidesServerData} from 'sentry/components/assistant/types';
 import ConfigStore from 'sentry/stores/configStore';
 import {trackAnalyticsEvent} from 'sentry/utils/analytics';
+import {
+  cleanupActiveRefluxSubscriptions,
+  makeSafeRefluxStore,
+  SafeRefluxStore,
+  SafeStoreDefinition,
+} from 'sentry/utils/makeSafeRefluxStore';
 
 function guidePrioritySort(a: Guide, b: Guide) {
   const a_priority = a.priority ?? Number.MAX_SAFE_INTEGER;
@@ -66,8 +72,9 @@ const defaultState: GuideStoreState = {
 };
 
 type GuideStoreInterface = {
-  onFetchSucceeded(data: GuidesServerData): void;
+  browserHistoryListener: null | (() => void);
 
+  onFetchSucceeded(data: GuidesServerData): void;
   onRegisterAnchor(target: string): void;
   onUnregisterAnchor(target: string): void;
   recordCue(guide: string): void;
@@ -75,23 +82,45 @@ type GuideStoreInterface = {
   updatePrevGuide(nextGuide: Guide | null): void;
 };
 
-const storeConfig: Reflux.StoreDefinition & GuideStoreInterface = {
+const storeConfig: Reflux.StoreDefinition & GuideStoreInterface & SafeStoreDefinition = {
   state: defaultState,
+  unsubscribeListeners: [],
+  browserHistoryListener: null,
 
   init() {
     this.state = defaultState;
 
     this.api = new Client();
-    this.listenTo(GuideActions.fetchSucceeded, this.onFetchSucceeded);
-    this.listenTo(GuideActions.closeGuide, this.onCloseGuide);
-    this.listenTo(GuideActions.nextStep, this.onNextStep);
-    this.listenTo(GuideActions.toStep, this.onToStep);
-    this.listenTo(GuideActions.registerAnchor, this.onRegisterAnchor);
-    this.listenTo(GuideActions.unregisterAnchor, this.onUnregisterAnchor);
-    this.listenTo(OrganizationsActions.setActive, this.onSetActiveOrganization);
+
+    this.unsubscribeListeners.push(
+      this.listenTo(GuideActions.fetchSucceeded, this.onFetchSucceeded)
+    );
+    this.unsubscribeListeners.push(
+      this.listenTo(GuideActions.closeGuide, this.onCloseGuide)
+    );
+    this.unsubscribeListeners.push(this.listenTo(GuideActions.nextStep, this.onNextStep));
+    this.unsubscribeListeners.push(this.listenTo(GuideActions.toStep, this.onToStep));
+    this.unsubscribeListeners.push(
+      this.listenTo(GuideActions.registerAnchor, this.onRegisterAnchor)
+    );
+    this.unsubscribeListeners.push(
+      this.listenTo(GuideActions.unregisterAnchor, this.onUnregisterAnchor)
+    );
+    this.unsubscribeListeners.push(
+      this.listenTo(OrganizationsActions.setActive, this.onSetActiveOrganization)
+    );
 
     window.addEventListener('load', this.onURLChange, false);
-    browserHistory.listen(() => this.onURLChange());
+    this.browserHistoryListener = browserHistory.listen(() => this.onURLChange());
+  },
+
+  teardown() {
+    cleanupActiveRefluxSubscriptions(this.unsubscribeListeners);
+    window.removeEventListener('load', this.onURLChange);
+
+    if (this.browserHistoryListener) {
+      this.browserHistoryListener();
+    }
   },
 
   onURLChange() {
@@ -250,6 +279,8 @@ const storeConfig: Reflux.StoreDefinition & GuideStoreInterface = {
   },
 };
 
-const GuideStore = Reflux.createStore(storeConfig) as Reflux.Store & GuideStoreInterface;
+const GuideStore = Reflux.createStore(
+  makeSafeRefluxStore(storeConfig)
+) as SafeRefluxStore & GuideStoreInterface;
 
 export default GuideStore;

+ 6 - 2
static/app/stores/hookStore.tsx

@@ -2,6 +2,7 @@ import isUndefined from 'lodash/isUndefined';
 import Reflux from 'reflux';
 
 import {HookName, Hooks} from 'sentry/types/hooks';
+import {makeSafeRefluxStore} from 'sentry/utils/makeSafeRefluxStore';
 
 type HookStoreInterface = {
   add<H extends HookName>(hookName: H, callback: Hooks[H]): void;
@@ -26,7 +27,7 @@ const storeConfig: Reflux.StoreDefinition & HookStoreInterface = {
       this.hooks[hookName] = [];
     }
 
-    this.hooks[hookName]!.push(callback);
+    this.hooks[hookName].push(callback);
     this.trigger(hookName, this.hooks[hookName]);
   },
 
@@ -49,6 +50,9 @@ const storeConfig: Reflux.StoreDefinition & HookStoreInterface = {
  *
  * This functionality is primarily used by the SASS sentry.io product.
  */
-const HookStore = Reflux.createStore(storeConfig) as Reflux.Store & HookStoreInterface;
+
+const HookStore = Reflux.createStore(
+  makeSafeRefluxStore(storeConfig)
+) as HookStoreInterface & Reflux.Store;
 
 export default HookStore;

Some files were not shown because too many files changed in this diff