Clear dispatch token

It seems that in certain versions of React it was possible for `componentWillMount` to be called more than once. facebook/react#6613 If that happened to a `connect`ed component, it would cause `dispatcher.unregister` to be called again with a token that was already unregistered causing an exception. This `null`s the token field so unregister can only be called once for a given token.
HubSpot · Mar 6, 2017 · 6fe0eb6 · 6fe0eb6
1 parent e16326d
commit 6fe0eb6
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 60 deletions.
diff --git a/src/dependencies/StoreDependencyMixin.js b/src/dependencies/StoreDependencyMixin.js
@@ -16,20 +16,17 @@ import { enforceDispatcher } from '../dispatcher/DispatcherInterface';
 import invariant from 'invariant';
 
 type ReactMixin = {
-  __dispatchToken?: string;
-  propTypes?: Object;
-  componentWillMount: Function;
-  componentWillReceiveProps: Function;
-  componentDidUpdate?: Function;
-  componentWillUnmount: Function;
-}
+  __dispatchToken?: string,
+  propTypes?: Object,
+  componentWillMount: Function,
+  componentWillReceiveProps: Function,
+  componentDidUpdate?: Function,
+  componentWillUnmount: Function,
+};
 
-function onlyStoreStateChanged(dependencies, state, prevState): bool {
+function onlyStoreStateChanged(dependencies, state, prevState): boolean {
   for (const field in state) {
-    if (
-      !dependencies.hasOwnProperty(field) &&
-      state[field] !== prevState[field]
-    ) {
+    if (!dependencies.hasOwnProperty(field) && state[field] !== prevState[field]) {
       return false;
     }
   }
@@ -38,7 +35,7 @@ function onlyStoreStateChanged(dependencies, state, prevState): bool {
 
 export default function StoreDependencyMixin(
   dependencies: DependencyMap,
-  dispatcher: ?Dispatcher = DispatcherInstance.get()
+  dispatcher: ?Dispatcher = DispatcherInstance.get(),
 ): ReactMixin {
   enforceDispatcher(dispatcher);
 
@@ -53,46 +50,36 @@ export default function StoreDependencyMixin(
       invariant(
         this.__dispatchToken === undefined,
         'Only one `StoreDependencyMixin` may be registered on `%s`',
-        this.constructor.displayName
+        this.constructor.displayName,
       );
       if (dispatcher) {
         this.__dispatchToken = dispatcher.register(
-          handleDispatch.bind(
-            null,
-            dispatcher,
-            dependencyIndex,
-            (entry) => {
-              this.setState(
-                calculateForDispatch(dependencies, entry, this.props, this.state)
-              );
-            }
-          )
+          handleDispatch.bind(null, dispatcher, dependencyIndex, entry => {
+            this.setState(calculateForDispatch(dependencies, entry, this.props, this.state));
+          }),
         );
       }
     },
 
     componentWillReceiveProps(nextProps): void {
-      this.setState(
-        calculateForPropsChange(dependencies, nextProps, this.state)
-      );
+      this.setState(calculateForPropsChange(dependencies, nextProps, this.state));
     },
 
     componentWillUnmount(): void {
-      if (dispatcher && this.__dispatchToken) {
-        dispatcher.unregister(this.__dispatchToken);
+      const dispatchToken = this.__dispatchToken;
+      if (dispatcher && dispatchToken) {
+        this.__dispatchToken = null;
+        dispatcher.unregister(dispatchToken);
       }
     },
-
   };
 
   if (dependenciesUseState(dependencies)) {
     mixin.componentDidUpdate = function componentDidUpdate(_, prevState) {
       if (onlyStoreStateChanged(dependencies, this.state, prevState)) {
         return;
       }
-      this.setState(
-        calculateForStateChange(dependencies, this.props, this.state)
-      );
+      this.setState(calculateForStateChange(dependencies, this.props, this.state));
     };
   }
 

diff --git a/src/dependencies/connect.js b/src/dependencies/connect.js
@@ -1,23 +1,23 @@
 /* @flow */
-import type {DependencyIndexEntry, DependencyMap} from './DependencyMap';
-import type {Dispatcher} from 'flux';
+import type { DependencyIndexEntry, DependencyMap } from './DependencyMap';
+import type { Dispatcher } from 'flux';
 import {
   calculateInitial,
   calculateForDispatch,
   calculateForPropsChange,
   dependencyPropTypes,
-  makeDependencyIndex
+  makeDependencyIndex,
 } from '../dependencies/DependencyMap';
-import {handleDispatch} from './Dispatch';
-import {get as getDispatcherInstance} from '../dispatcher/DispatcherInstance';
-import {enforceDispatcher} from '../dispatcher/DispatcherInterface';
-import React, {Component} from 'react';
+import { handleDispatch } from './Dispatch';
+import { get as getDispatcherInstance } from '../dispatcher/DispatcherInstance';
+import { enforceDispatcher } from '../dispatcher/DispatcherInterface';
+import React, { Component } from 'react';
 
 function transferStaticProperties(
   fromClass: Object,
   // By setting the type to Object, I'm doing a little dance around the type
   // checker... I fully expect this to break after a future flow upgrade.
-  toClass: Object
+  toClass: Object,
 ) {
   Object.keys(fromClass).forEach(staticField => {
     toClass[staticField] = fromClass[staticField];
@@ -26,7 +26,7 @@ function transferStaticProperties(
 
 export default function connect(
   dependencies: DependencyMap,
-  dispatcher: ?Dispatcher = getDispatcherInstance()
+  dispatcher: ?Dispatcher = getDispatcherInstance(),
 ): Function {
   enforceDispatcher(dispatcher);
 
@@ -47,33 +47,26 @@ export default function connect(
       componentWillMount() {
         if (dispatcher) {
           this.dispatchToken = dispatcher.register(
-            handleDispatch.bind(
-              null,
-              dispatcher,
-              dependencyIndex,
-              this.handleDispatch.bind(this)
-            )
+            handleDispatch.bind(null, dispatcher, dependencyIndex, this.handleDispatch.bind(this)),
           );
         }
         this.setState(calculateInitial(dependencies, this.props, this.state));
       }
 
       componentWillReceiveProps(nextProps: Object): void {
-        this.setState(
-          calculateForPropsChange(dependencies, nextProps, this.state)
-        );
+        this.setState(calculateForPropsChange(dependencies, nextProps, this.state));
       }
 
       componentWillUnmount(): void {
-        if (dispatcher && this.dispatchToken) {
-          dispatcher.unregister(this.dispatchToken);
+        const dispatchToken = this.dispatchToken;
+        if (dispatcher && dispatchToken) {
+          this.dispatchToken = null;
+          dispatcher.unregister(dispatchToken);
         }
       }
 
       handleDispatch(entry: DependencyIndexEntry) {
-        this.setState(
-          calculateForDispatch(dependencies, entry, this.props, this.state)
-        );
+        this.setState(calculateForDispatch(dependencies, entry, this.props, this.state));
       }
 
       render() {
@@ -83,10 +76,7 @@ export default function connect(
 
     transferStaticProperties(BaseComponent, ConnectedComponent);
     ConnectedComponent.displayName = `Connected(${BaseComponent.displayName})`;
-    ConnectedComponent.propTypes = dependencyPropTypes(
-      dependencies,
-      BaseComponent.propTypes
-    );
+    ConnectedComponent.propTypes = dependencyPropTypes(dependencies, BaseComponent.propTypes);
 
     return ConnectedComponent;
   };