Dispatch can be called multiple times per tick (#1313)

Mostly fixes #1206

jest-setup.js

@@ -3,6 +3,8 @@
  * eslint-env jest 
  */
 
+import React from 'react';
+
 // See https://github.com/facebook/jest/issues/2208
 jest.mock('Linking', () => ({
   addEventListener: jest.fn(),
@@ -24,5 +26,20 @@ jest.mock('ScrollView', () => {
   return ScrollView;
 });
 
+// Mock setState so it waits using setImmediate before actually being called,
+// so we can use jest's mock timers to control it.
+// setState in the test renderer is sync instead of async like react and react-native.
+// This doesn't work with our NavigationContainer tests which test react-navigation's
+// behaviour against the async nature of setState.
+const setState = React.Component.prototype.setState;
+// $FlowExpectedError
+Object.defineProperty(React.Component.prototype, 'setState', {
+  value: function() {
+    setImmediate(() => {
+      setState.apply(this, arguments);
+    });
+  },
+});
+
 // $FlowExpectedError
 Date.now = jest.fn(() => 0);

src/__tests__/NavigationContainer-test.js

@@ -0,0 +1,203 @@
+/* @flow */
+
+import React from 'react';
+import 'react-native';
+
+import renderer from 'react-test-renderer';
+
+import NavigationActions from '../NavigationActions';
+import StackNavigator from '../navigators/StackNavigator';
+
+const FooScreen = () => <div />;
+const BarScreen = () => <div />;
+const BazScreen = () => <div />;
+const CarScreen = () => <div />;
+const DogScreen = () => <div />;
+const ElkScreen = () => <div />;
+const NavigationContainer = StackNavigator(
+  {
+    foo: {
+      screen: FooScreen,
+    },
+    bar: {
+      screen: BarScreen,
+    },
+    baz: {
+      screen: BazScreen,
+    },
+    car: {
+      screen: CarScreen,
+    },
+    dog: {
+      screen: DogScreen,
+    },
+    elk: {
+      screen: ElkScreen,
+    },
+  },
+  {
+    initialRouteName: 'foo',
+  }
+);
+
+jest.useFakeTimers();
+
+describe('NavigationContainer', () => {
+  describe('state.nav', () => {
+    it("should be preloaded with the router's initial state", () => {
+      const navigationContainer = renderer
+        .create(<NavigationContainer />)
+        .getInstance();
+      expect(navigationContainer.state.nav).toMatchObject({ index: 0 });
+      expect(navigationContainer.state.nav.routes).toBeInstanceOf(Array);
+      expect(navigationContainer.state.nav.routes.length).toBe(1);
+      expect(navigationContainer.state.nav.routes[0]).toMatchObject({
+        routeName: 'foo',
+      });
+    });
+  });
+
+  describe('dispatch', () => {
+    it('returns true when given a valid action', () => {
+      const navigationContainer = renderer
+        .create(<NavigationContainer />)
+        .getInstance();
+      jest.runOnlyPendingTimers();
+      expect(
+        navigationContainer.dispatch(
+          NavigationActions.navigate({ routeName: 'bar' })
+        )
+      ).toEqual(true);
+    });
+
+    it('returns false when given an invalid action', () => {
+      const navigationContainer = renderer
+        .create(<NavigationContainer />)
+        .getInstance();
+      jest.runOnlyPendingTimers();
+      expect(navigationContainer.dispatch(NavigationActions.back())).toEqual(
+        false
+      );
+    });
+
+    it('updates state.nav with an action by the next tick', () => {
+      const navigationContainer = renderer
+        .create(<NavigationContainer />)
+        .getInstance();
+
+      expect(
+        navigationContainer.dispatch(
+          NavigationActions.navigate({ routeName: 'bar' })
+        )
+      ).toEqual(true);
+
+      // Fake the passing of a tick
+      jest.runOnlyPendingTimers();
+
+      expect(navigationContainer.state.nav).toMatchObject({
+        index: 1,
+        routes: [{ routeName: 'foo' }, { routeName: 'bar' }],
+      });
+    });
+
+    it('does not discard actions when called twice in one tick', () => {
+      const navigationContainer = renderer
+        .create(<NavigationContainer />)
+        .getInstance();
+      const initialState = JSON.parse(
+        JSON.stringify(navigationContainer.state.nav)
+      );
+
+      // First dispatch
+      expect(
+        navigationContainer.dispatch(
+          NavigationActions.navigate({ routeName: 'bar' })
+        )
+      ).toEqual(true);
+
+      // Make sure that the test runner has NOT synchronously applied setState before the tick
+      expect(navigationContainer.state.nav).toMatchObject(initialState);
+
+      // Second dispatch
+      expect(
+        navigationContainer.dispatch(
+          NavigationActions.navigate({ routeName: 'baz' })
+        )
+      ).toEqual(true);
+
+      // Fake the passing of a tick
+      jest.runOnlyPendingTimers();
+
+      expect(navigationContainer.state.nav).toMatchObject({
+        index: 2,
+        routes: [
+          { routeName: 'foo' },
+          { routeName: 'bar' },
+          { routeName: 'baz' },
+        ],
+      });
+    });
+
+    it('does not discard actions when called more than 2 times in one tick', () => {
+      const navigationContainer = renderer
+        .create(<NavigationContainer />)
+        .getInstance();
+      const initialState = JSON.parse(
+        JSON.stringify(navigationContainer.state.nav)
+      );
+
+      // First dispatch
+      expect(
+        navigationContainer.dispatch(
+          NavigationActions.navigate({ routeName: 'bar' })
+        )
+      ).toEqual(true);
+
+      // Make sure that the test runner has NOT synchronously applied setState before the tick
+      expect(navigationContainer.state.nav).toMatchObject(initialState);
+
+      // Second dispatch
+      expect(
+        navigationContainer.dispatch(
+          NavigationActions.navigate({ routeName: 'baz' })
+        )
+      ).toEqual(true);
+
+      // Third dispatch
+      expect(
+        navigationContainer.dispatch(
+          NavigationActions.navigate({ routeName: 'car' })
+        )
+      ).toEqual(true);
+
+      // Fourth dispatch
+      expect(
+        navigationContainer.dispatch(
+          NavigationActions.navigate({ routeName: 'dog' })
+        )
+      ).toEqual(true);
+
+      // Fifth dispatch
+      expect(
+        navigationContainer.dispatch(
+          NavigationActions.navigate({ routeName: 'elk' })
+        )
+      ).toEqual(true);
+
+      // Fake the passing of a tick
+      jest.runOnlyPendingTimers();
+
+      expect(navigationContainer.state.nav).toMatchObject({
+        index: 5,
+        routes: [
+          { routeName: 'foo' },
+          { routeName: 'bar' },
+          { routeName: 'baz' },
+          { routeName: 'car' },
+          { routeName: 'dog' },
+          { routeName: 'elk' },
+        ],
+      });
+    });
+  });
+});

src/createNavigationContainer.js

@@ -145,6 +145,13 @@ export default function createNavigationContainer<A: *, O: *>(
       this._validateProps(nextProps);
     }
 
+    componentDidUpdate() {
+      // Clear cached _nav every tick
+      if (this._nav === this.state.nav) {
+        this._nav = null;
+      }
+    }
+
     componentDidMount() {
       if (!this._isStateful()) {
         return;
@@ -166,6 +173,9 @@ export default function createNavigationContainer<A: *, O: *>(
       this.subs && this.subs.remove();
     }
 
+    // Per-tick temporary storage for state.nav
+    _nav: ?NavigationState;
+
     dispatch = (inputAction: PossiblyDeprecatedNavigationAction) => {
       // $FlowFixMe remove after we deprecate the old actions
       const action: A = NavigationActions.mapDeprecatedActionAndWarn(
@@ -174,10 +184,13 @@ export default function createNavigationContainer<A: *, O: *>(
       if (!this._isStateful()) {
         return false;
       }
-      const oldNav = this.state.nav;
+      this._nav = this._nav || this.state.nav;
+      const oldNav = this._nav;
       invariant(oldNav, 'should be set in constructor if stateful');
       const nav = Component.router.getStateForAction(action, oldNav);
       if (nav && nav !== oldNav) {
+        // Cache updates to state.nav during the tick to ensure that subsequent calls will not discard this change
+        this._nav = nav;
         this.setState({ nav }, () =>
           this._onNavigationStateChange(oldNav, nav, action)
         );