Cache event subscribers and clean up after unmounting (#3648)

* This caches "child event subscribers" by key and removes them when they're not needed anymore. Once a navigator unmounts we also remove upstream subscribers.

* Fix tests

src/__tests__/getChildEventSubscriber-test.js

@@ -11,10 +11,8 @@ test('child action events only flow when focused', () => {
   };
   const subscriptionRemove = () => {};
   parentSubscriber.mockReturnValueOnce({ remove: subscriptionRemove });
-  const childEventSubscriber = getChildEventSubscriber(
-    parentSubscriber,
-    'key1'
-  );
+  const childEventSubscriber = getChildEventSubscriber(parentSubscriber, 'key1')
+    .addListener;
   const testState = {
     key: 'foo',
     routeName: 'FooRoute',
@@ -66,11 +64,9 @@ test('grandchildren subscription', () => {
   const parentSubscriber = getChildEventSubscriber(
     grandParentSubscriber,
     'parent'
-  );
-  const childEventSubscriber = getChildEventSubscriber(
-    parentSubscriber,
-    'key1'
-  );
+  ).addListener;
+  const childEventSubscriber = getChildEventSubscriber(parentSubscriber, 'key1')
+    .addListener;
   const parentBlurState = {
     key: 'foo',
     routeName: 'FooRoute',
@@ -135,11 +131,9 @@ test('grandchildren transitions', () => {
   const parentSubscriber = getChildEventSubscriber(
     grandParentSubscriber,
     'parent'
-  );
-  const childEventSubscriber = getChildEventSubscriber(
-    parentSubscriber,
-    'key1'
-  );
+  ).addListener;
+  const childEventSubscriber = getChildEventSubscriber(parentSubscriber, 'key1')
+    .addListener;
   const makeFakeState = (childIndex, childIsTransitioning) => ({
     index: 1,
     isTransitioning: false,
@@ -230,11 +224,9 @@ test('grandchildren pass through transitions', () => {
   const parentSubscriber = getChildEventSubscriber(
     grandParentSubscriber,
     'parent'
-  );
-  const childEventSubscriber = getChildEventSubscriber(
-    parentSubscriber,
-    'key1'
-  );
+  ).addListener;
+  const childEventSubscriber = getChildEventSubscriber(parentSubscriber, 'key1')
+    .addListener;
   const makeFakeState = (childIndex, childIsTransitioning) => ({
     index: childIndex,
     isTransitioning: childIsTransitioning,
@@ -322,10 +314,8 @@ test('child focus with transition', () => {
   };
   const subscriptionRemove = () => {};
   parentSubscriber.mockReturnValueOnce({ remove: subscriptionRemove });
-  const childEventSubscriber = getChildEventSubscriber(
-    parentSubscriber,
-    'key1'
-  );
+  const childEventSubscriber = getChildEventSubscriber(parentSubscriber, 'key1')
+    .addListener;
   const randomAction = { type: 'FooAction' };
   const testState = {
     key: 'foo',
@@ -417,10 +407,8 @@ test('child focus with immediate transition', () => {
   };
   const subscriptionRemove = () => {};
   parentSubscriber.mockReturnValueOnce({ remove: subscriptionRemove });
-  const childEventSubscriber = getChildEventSubscriber(
-    parentSubscriber,
-    'key1'
-  );
+  const childEventSubscriber = getChildEventSubscriber(parentSubscriber, 'key1')
+    .addListener;
   const randomAction = { type: 'FooAction' };
   const testState = {
     key: 'foo',

src/getChildEventSubscriber.js

@@ -11,6 +11,18 @@ export default function getChildEventSubscriber(addListener, key) {
   const willBlurSubscribers = new Set();
   const didBlurSubscribers = new Set();
 
+  const removeAll = () => {
+    [
+      actionSubscribers,
+      willFocusSubscribers,
+      didFocusSubscribers,
+      willBlurSubscribers,
+      didBlurSubscribers,
+    ].forEach(set => set.clear());
+
+    upstreamSubscribers.forEach(subs => subs && subs.remove());
+  };
+
   const getChildSubscribers = evtName => {
     switch (evtName) {
       case 'action':
@@ -43,10 +55,6 @@ export default function getChildEventSubscriber(addListener, key) {
   // considered blurred
   let lastEmittedEvent = 'didBlur';
 
-  const cleanup = () => {
-    upstreamSubscribers.forEach(subs => subs && subs.remove());
-  };
-
   const upstreamEvents = [
     'willFocus',
     'didFocus',
@@ -133,15 +141,18 @@ export default function getChildEventSubscriber(addListener, key) {
     })
   );
 
-  return (eventName, eventHandler) => {
-    const subscribers = getChildSubscribers(eventName);
-    if (!subscribers) {
-      throw new Error(`Invalid event name "${eventName}"`);
-    }
-    subscribers.add(eventHandler);
-    const remove = () => {
-      subscribers.delete(eventHandler);
-    };
-    return { remove };
+  return {
+    removeAll,
+    addListener(eventName, eventHandler) {
+      const subscribers = getChildSubscribers(eventName);
+      if (!subscribers) {
+        throw new Error(`Invalid event name "${eventName}"`);
+      }
+      subscribers.add(eventHandler);
+      const remove = () => {
+        subscribers.delete(eventHandler);
+      };
+      return { remove };
+    },
   };
 }

src/navigators/createNavigator.js

@@ -8,6 +8,24 @@ function createNavigator(NavigatorView, router, navigationConfig) {
     static router = router;
     static navigationOptions = null;
 
+    childEventSubscribers = {};
+
+    // Cleanup subscriptions for routes that no longer exist
+    componentDidUpdate() {
+      const activeKeys = this.props.navigation.state.routes.map(r => r.key);
+      Object.keys(this.childEventSubscribers).forEach(key => {
+        if (!activeKeys.includes(key)) {
+          this.childEventSubscribers[key].removeAll();
+          delete this.childEventSubscribers[key];
+        }
+      });
+    }
+
+    // Remove all subscriptions
+    componentWillUnmount() {
+      Object.values(this.childEventSubscribers).map(s => s.removeAll());
+    }
+
     render() {
       const { navigation, screenProps } = this.props;
       const { dispatch, state, addListener } = navigation;
@@ -18,10 +36,17 @@ function createNavigator(NavigatorView, router, navigationConfig) {
         const getComponent = () =>
           router.getComponentForRouteName(route.routeName);
 
+        if (!this.childEventSubscribers[route.key]) {
+          this.childEventSubscribers[route.key] = getChildEventSubscriber(
+            addListener,
+            route.key
+          );
+        }
+
         const childNavigation = addNavigationHelpers({
           dispatch,
           state: route,
-          addListener: getChildEventSubscriber(addListener, route.key),
+          addListener: this.childEventSubscribers[route.key].addListener,
         });
         const options = router.getScreenOptions(childNavigation, screenProps);
         descriptors[route.key] = {

src/routers/createConfigGetter.js

@@ -2,7 +2,6 @@ import invariant from '../utils/invariant';
 
 import getScreenForRouteName from './getScreenForRouteName';
 import validateScreenOptions from './validateScreenOptions';
-import getChildEventSubscriber from '../getChildEventSubscriber';
 
 function applyConfig(configurer, navigationOptions, configProps) {
   if (typeof configurer === 'function') {

src/views/Drawer/DrawerView.js

@@ -4,7 +4,6 @@ import DrawerLayout from 'react-native-drawer-layout-polyfill';
 
 import addNavigationHelpers from '../../addNavigationHelpers';
 import DrawerSidebar from './DrawerSidebar';
-import getChildEventSubscriber from '../../getChildEventSubscriber';
 
 /**
  * Component that renders the drawer.