Merge remote-tracking branch 'upstream/next' into patch-1

Author: pelotom

docs/advanced/Middleware.md

@@ -265,12 +265,16 @@ The implementation of [`applyMiddleware()`](../api/applyMiddleware.md) that ship
 
 * It only exposes a subset of the [store API](../api/Store.md) to the middleware: [`dispatch(action)`](../api/Store.md#dispatch) and [`getState()`](../api/Store.md#getState).
 
-* It does a bit of trickery to make sure that if you call `store.dispatch(action)` from your middleware instead of `next(action)`, the action will actually travel the whole middleware chain again, including the current middleware. This is useful for asynchronous middleware, as we have seen [previously](AsyncActions.md).
+* It does a bit of trickery to make sure that if you call `store.dispatch(action)` from your middleware instead of `next(action)`, the action will actually travel the whole middleware chain again, including the current middleware. This is useful for asynchronous middleware, as we have seen [previously](AsyncActions.md). There is one caveat when calling `dispatch` during setup, described below.
 
 * To ensure that you may only apply middleware once, it operates on `createStore()` rather than on `store` itself. Instead of `(store, middlewares) => store`, its signature is `(...middlewares) => (createStore) => createStore`.
 
 Because it is cumbersome to apply functions to `createStore()` before using it, `createStore()` accepts an optional last argument to specify such functions.
 
+#### Caveat: Dispatching During Setup
+
+While `applyMiddleware` executes and sets up your middleware, the `store.dispatch` function will point to the vanilla version provided by `createStore`. Dispatching would result in no other middleware being applied. If you are expecting an interaction with another middleware during setup, you will probably be disappointed. Because of this unexpected behavior, `applyMiddleware` will throw an error if you try to dispatch an action before the set up completes. Instead, you should either communicate directly with that other middleware via a common object (for an API-calling middleware, this may be your API client object) or waiting until after the middleware is constructed with a callback. 
+
 ### The Final Approach
 
 Given this middleware we just wrote:

index.d.ts

@@ -46,7 +46,7 @@ export interface Action<T = any> {
  * @template S The type of state consumed and produced by this reducer.
  * @template A The type of actions the reducer can potentially respond to.
  */
-export type Reducer<S = {}, A extends Action = Action> = (state: S, action: A) => S;
+export type Reducer<S = {}, A extends Action = Action> = (state: S | undefined, action: A) => S;
 
 /**
  * Object whose values correspond to different reducer functions.

package.json

@@ -4,7 +4,6 @@
   "description": "Predictable state container for JavaScript apps",
   "main": "lib/index.js",
   "module": "es/index.js",
-  "jsnext:main": "es/index.js",
   "typings": "./index.d.ts",
   "files": [
     "dist",

src/applyMiddleware.js

@@ -19,7 +19,12 @@ import compose from './compose'
 export default function applyMiddleware(...middlewares) {
   return (createStore) => (reducer, preloadedState, enhancer) => {
     const store = createStore(reducer, preloadedState, enhancer)
-    let dispatch = store.dispatch
+    let dispatch = () => {
+      throw new Error(
+        `Dispatching while constructing your middleware is not allowed. ` +
+        `Other middleware would not be applied to this dispatch.`
+      )
+    }
     let chain = []
 
     const middlewareAPI = {

src/combineReducers.js

@@ -1,4 +1,4 @@
-import { ActionTypes } from './createStore'
+import ActionTypes from './utils/actionTypes'
 import isPlainObject from 'lodash/isPlainObject'
 import warning from './utils/warning'
 

src/createStore.js

@@ -1,15 +1,7 @@
 import isPlainObject from 'lodash/isPlainObject'
 import $$observable from 'symbol-observable'
 
-/**
- * These are private action types reserved by Redux.
- * For any unknown actions, you must return the current state.
- * If the current state is undefined, you must return the initial state.
- * Do not reference these action types directly in your code.
- */
-export const ActionTypes = {
-  INIT: '@@redux/INIT'
-}
+import ActionTypes from './utils/actionTypes'
 
 /**
  * Creates a Redux store that holds the state tree.
@@ -72,6 +64,14 @@ export default function createStore(reducer, preloadedState, enhancer) {
    * @returns {any} The current state tree of your application.
    */
   function getState() {
+    if (isDispatching) {
+      throw new Error(
+        'You may not call store.getState() while the reducer is executing. ' +
+        'The reducer has already received the state as an argument. ' +
+        'Pass it down from the top reducer instead of reading it from the store.'
+      )
+    }
+
     return currentState
   }
 
@@ -103,6 +103,15 @@ export default function createStore(reducer, preloadedState, enhancer) {
       throw new Error('Expected listener to be a function.')
     }
 
+    if (isDispatching) {
+      throw new Error(
+        'You may not call store.subscribe() while the reducer is executing. ' +
+        'If you would like to be notified after the store has been updated, subscribe from a ' +
+        'component and invoke store.getState() in the callback to access the latest state. ' +
+        'See http://redux.js.org/docs/api/Store.html#subscribe for more details.'
+      )
+    }
+
     let isSubscribed = true
 
     ensureCanMutateNextListeners()
@@ -113,6 +122,13 @@ export default function createStore(reducer, preloadedState, enhancer) {
         return
       }
 
+      if (isDispatching) {
+        throw new Error(
+          'You may not unsubscribe from a store listener while the reducer is executing. ' +
+          'See http://redux.js.org/docs/api/Store.html#subscribe for more details.'
+        )
+      }
+
       isSubscribed = false
 
       ensureCanMutateNextListeners()

src/utils/actionTypes.js

@@ -0,0 +1,11 @@
+/**
+ * These are private action types reserved by Redux.
+ * For any unknown actions, you must return the current state.
+ * If the current state is undefined, you must return the initial state.
+ * Do not reference these action types directly in your code.
+ */
+var ActionTypes = {
+  INIT: '@@redux/INIT'
+}
+
+export default ActionTypes

test/applyMiddleware.spec.js

@@ -4,6 +4,17 @@ import { addTodo, addTodoAsync, addTodoIfEmpty } from './helpers/actionCreators'
 import { thunk } from './helpers/middleware'
 
 describe('applyMiddleware', () => {
+  it('warns when dispatching during middleware setup', () => {
+    function dispatchingMiddleware(store) {
+      store.dispatch(addTodo('Dont dispatch in middleware setup'))
+      return next => action => next(action)
+    }
+
+    expect(() =>
+      applyMiddleware(dispatchingMiddleware)(createStore)(reducers.todos)
+    ).toThrow()
+  })
+
   it('wraps dispatch method with middleware once', () => {
     function test(spyOnMethods) {
       return methods => {
@@ -91,42 +102,4 @@ describe('applyMiddleware', () => {
       done()
     })
   })
-
-    it('passes through all arguments of dispatch calls from within middleware', () => {
-        const spy = jest.fn()
-        const testCallArgs = ['test']
-        function multiArgMiddleware() {
-            return next => (action, callArgs) => {
-                if (Array.isArray(callArgs)) {
-                    return action(...callArgs)
-                }
-                return next(action)
-            }
-        }
-        function dummyMiddleware({ dispatch }) {
-            return next => action => dispatch(action, testCallArgs)
-        }
-
-        const store = createStore(reducers.todos, applyMiddleware(multiArgMiddleware, dummyMiddleware))
-        store.dispatch(spy)
-        expect(spy.mock.calls[0]).toEqual(testCallArgs)
-    })
-
-  it('keeps unwrapped dispatch available while middleware is initializing', () => {
-    // This is documenting the existing behavior in Redux 3.x.
-    // We plan to forbid this in Redux 4.x.
-
-    function earlyDispatch({ dispatch }) {
-      dispatch(addTodo('Hello'))
-      return () => action => action
-    }
-
-    const store = createStore(reducers.todos, applyMiddleware(earlyDispatch))
-    expect(store.getState()).toEqual([
-      {
-        id: 1,
-        text: 'Hello'
-      }
-    ])
-  })
 })

test/combineReducers.spec.js

@@ -1,6 +1,7 @@
 /* eslint-disable no-console */
 import { combineReducers } from '../src'
-import createStore, { ActionTypes } from '../src/createStore'
+import createStore from '../src/createStore'
+import ActionTypes from '../src/utils/actionTypes'
 
 describe('Utils', () => {
   describe('combineReducers', () => {

test/createStore.spec.js

@@ -1,5 +1,13 @@
 import { createStore, combineReducers } from '../src/index'
-import { addTodo, dispatchInMiddle, throwError, unknownAction } from './helpers/actionCreators'
+import { 
+  addTodo, 
+  dispatchInMiddle, 
+  getStateInMiddle,
+  subscribeInMiddle,
+  unsubscribeInMiddle,
+  throwError, 
+  unknownAction 
+} from './helpers/actionCreators'
 import * as reducers from './helpers/reducers'
 import * as Rx from 'rxjs'
 import $$observable from 'symbol-observable'
@@ -461,6 +469,31 @@ describe('createStore', () => {
     ).toThrow(/may not dispatch/)
   })
 
+  it('does not allow getState() from within a reducer', () => {
+    const store = createStore(reducers.getStateInTheMiddleOfReducer)
+
+    expect(() =>
+      store.dispatch(getStateInMiddle(store.getState.bind(store)))
+    ).toThrow(/You may not call store.getState()/)
+  })
+
+  it('does not allow subscribe() from within a reducer', () => {
+    const store = createStore(reducers.subscribeInTheMiddleOfReducer)
+
+    expect(() =>
+      store.dispatch(subscribeInMiddle(store.subscribe.bind(store, () => {})))
+    ).toThrow(/You may not call store.subscribe()/)
+  })
+
+  it('does not allow unsubscribe from subscribe() from within a reducer', () => {
+    const store = createStore(reducers.unsubscribeInTheMiddleOfReducer)
+    const unsubscribe = store.subscribe(() => {})
+
+    expect(() =>
+      store.dispatch(unsubscribeInMiddle(unsubscribe.bind(store)))
+    ).toThrow(/You may not unsubscribe from a store/)
+  })
+
   it('recovers from an error within a reducer', () => {
     const store = createStore(reducers.errorThrowingReducer)
     expect(() =>

test/helpers/actionCreators.js

@@ -1,4 +1,12 @@
-import { ADD_TODO, DISPATCH_IN_MIDDLE, THROW_ERROR, UNKNOWN_ACTION } from './actionTypes'
+import { 
+  ADD_TODO, 
+  DISPATCH_IN_MIDDLE, 
+  GET_STATE_IN_MIDDLE, 
+  SUBSCRIBE_IN_MIDDLE,
+  UNSUBSCRIBE_IN_MIDDLE, 
+  THROW_ERROR, 
+  UNKNOWN_ACTION 
+} from './actionTypes'
 
 export function addTodo(text) {
   return { type: ADD_TODO, text }
@@ -26,6 +34,27 @@ export function dispatchInMiddle(boundDispatchFn) {
   }
 }
 
+export function getStateInMiddle(boundGetStateFn) {
+  return {
+    type: GET_STATE_IN_MIDDLE,
+    boundGetStateFn
+  }
+}
+
+export function subscribeInMiddle(boundSubscribeFn) {
+  return {
+    type: SUBSCRIBE_IN_MIDDLE,
+    boundSubscribeFn
+  }
+}
+
+export function unsubscribeInMiddle(boundUnsubscribeFn) {
+  return {
+    type: UNSUBSCRIBE_IN_MIDDLE,
+    boundUnsubscribeFn
+  }
+}
+
 export function throwError() {
   return {
     type: THROW_ERROR

test/helpers/actionTypes.js

@@ -1,4 +1,7 @@
 export const ADD_TODO = 'ADD_TODO'
 export const DISPATCH_IN_MIDDLE = 'DISPATCH_IN_MIDDLE'
+export const GET_STATE_IN_MIDDLE = 'GET_STATE_IN_MIDDLE'
+export const SUBSCRIBE_IN_MIDDLE = 'SUBSCRIBE_IN_MIDDLE'
+export const UNSUBSCRIBE_IN_MIDDLE = 'UNSUBSCRIBE_IN_MIDDLE'
 export const THROW_ERROR = 'THROW_ERROR'
 export const UNKNOWN_ACTION = 'UNKNOWN_ACTION'

test/helpers/reducers.js

@@ -1,4 +1,11 @@
-import { ADD_TODO, DISPATCH_IN_MIDDLE, THROW_ERROR } from './actionTypes'
+import { 
+  ADD_TODO, 
+  DISPATCH_IN_MIDDLE, 
+  GET_STATE_IN_MIDDLE, 
+  SUBSCRIBE_IN_MIDDLE,
+  UNSUBSCRIBE_IN_MIDDLE,
+  THROW_ERROR 
+} from './actionTypes'
 
 
 function id(state = []) {
@@ -46,6 +53,36 @@ export function dispatchInTheMiddleOfReducer(state = [], action) {
   }
 }
 
+export function getStateInTheMiddleOfReducer(state = [], action) {
+  switch (action.type) {
+    case GET_STATE_IN_MIDDLE:
+      action.boundGetStateFn()
+      return state
+    default:
+      return state
+  }
+}
+
+export function subscribeInTheMiddleOfReducer(state = [], action) {
+  switch (action.type) {
+    case SUBSCRIBE_IN_MIDDLE:
+      action.boundSubscribeFn()
+      return state
+    default:
+      return state
+  }
+}
+
+export function unsubscribeInTheMiddleOfReducer(state = [], action) {
+  switch (action.type) {
+    case UNSUBSCRIBE_IN_MIDDLE:
+      action.boundUnsubscribeFn()
+      return state
+    default:
+      return state
+  }
+}
+
 export function errorThrowingReducer(state = [], action) {
   switch (action.type) {
     case THROW_ERROR:

test/typescript.spec.js

@@ -6,6 +6,9 @@ describe('TypeScript definitions', function () {
     tt.compileDirectory(
       __dirname + '/typescript',
       fileName => fileName.match(/\.ts$/),
+      {
+        strictNullChecks: true
+      },
       () => done()
     )
   })

test/typescript/actionCreators.ts

@@ -1,7 +1,7 @@
 import {
   ActionCreator, Action, Dispatch,
   bindActionCreators, ActionCreatorsMapObject
-} from "../../index";
+} from "../../"
 
 
 interface AddTodoAction extends Action {

test/typescript/actions.ts

@@ -1,4 +1,4 @@
-import {Action as ReduxAction} from "../../index";
+import {Action as ReduxAction} from "../../"
 
 
 namespace FSA {

test/typescript/compose.ts

@@ -1,4 +1,4 @@
-import {compose} from "../../index";
+import {compose} from "../../"
 
 // copied from DefinitelyTyped/compose-function
 

test/typescript/dispatch.ts

@@ -1,12 +1,12 @@
-import {Dispatch, Action} from "../../index";
+import {Dispatch, Action} from "../../"
 
 
 declare const dispatch: Dispatch<any>;
 
 const dispatchResult: Action = dispatch({type: 'TYPE'});
 
 // thunk
-declare module "../../index" {
+declare module "../../" {
     export interface Dispatch<S> {
         <R>(asyncAction: (dispatch: Dispatch<S>, getState: () => S) => R): R;
     }