Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forbid getState() and subscribe() while the reducer is being called #1568

Closed
gaearon opened this issue Apr 2, 2016 · 5 comments · Fixed by #1569
Closed

Forbid getState() and subscribe() while the reducer is being called #1568

gaearon opened this issue Apr 2, 2016 · 5 comments · Fixed by #1569

Comments

@gaearon
Copy link
Contributor

gaearon commented Apr 2, 2016

This is an anti-pattern, first found here: reduxjs/redux-devtools#264.

I think we should protect against store.getState() calls from inside the reducer. They make the reducer impure, and you can always pass the store state down as the third argument if this is something you’re interested in.

I think we should forbid this pattern just like we forbid dispatch() while the reducer is running.

Same goes for subscribe() which should never be called from inside a reducer.

A pull request to implement this is welcome.

@mwilc0x
Copy link
Contributor

mwilc0x commented Apr 2, 2016

I can work on this.

@gaearon
Copy link
Contributor Author

gaearon commented Apr 2, 2016

👍 Don’t forget the unsubscribe too (the function being returned from subscribe()).

@gaearon
Copy link
Contributor Author

gaearon commented Apr 2, 2016

Thanks! Let’s continue the discussion in #1569.

@zackster
Copy link

zackster commented Feb 17, 2020

@gaearon I realize that this has been resolved, but since this is the clear Google result when people search this issue, I feel like this is an appropriate place to make this comment.

First off, thank you for your work on Redux.

You mention "They make the reducer impure, and you can always pass the store state down as the third argument if this is something you’re interested in."

Could you explain where this third argument might be invoked?

Citing the official documentation on using combineReducers, it seems like there's no clear way to pass a third parameter.

The next article in the documentation, titled "Beyond combineReducers", does suggest some patterns.

function combinedReducer(state, action) {
  switch (action.type) {
    case 'A_TYPICAL_ACTION': {
      return {
        a: sliceReducerA(state.a, action),
        b: sliceReducerB(state.b, action)
      }
    }
    case 'SOME_SPECIAL_ACTION': {
      return {
        // specifically pass state.b as an additional argument
        a: sliceReducerA(state.a, action, state.b),
        b: sliceReducerB(state.b, action)
      }
    }
    case 'ANOTHER_SPECIAL_ACTION': {
      return {
        a: sliceReducerA(state.a, action),
        // specifically pass the entire state as an additional argument
        b: sliceReducerB(state.b, action, state)
      }
    }
    default:
      return state
  }
}

The way I'm guessing this works (but it's not entirely clear) is that:

a) sliceReducerA and sliceReducerB are reducers that are included in the combineReducers invocation
b) In the combinedReducer invocation, this new combinedReducer is included as well
c) The third param is introduced in the function declaration of sliceReducerA, e.g., export default function sliceReducerA(state = initialState, action, wholeState).

Is this understanding correct? In practice, would anything need to be modified in the dispatch call or would the wholeState automatically be inferred?

The other way I tried it, passing around store.getState().otherSliceINeeded, was unclean in various ways, not to mention the fact that in some cases, it would require dispatching another action in sequence to update the otherSlice. [1]

[1] Although it seems like with the new batch public API, there are efficient ways of doing this.

@markerikson
Copy link
Contributor

@zackster : bit of a thread necro here :)

but yes, the point is that you'd need to break out of the default paradigm used by combineReducers in order to have access to the rest of the state if desired.

@reduxjs reduxjs locked as resolved and limited conversation to collaborators Feb 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants