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

Actions dispatched in componentWillMount and similar #167

Closed
leoasis opened this issue Oct 22, 2015 · 11 comments
Closed

Actions dispatched in componentWillMount and similar #167

leoasis opened this issue Oct 22, 2015 · 11 comments

Comments

@leoasis
Copy link

leoasis commented Oct 22, 2015

There is some kind on unwanted behavior when we have "unstable states", which are the states that cause the UI to be rendered, then mounted or whatever, and then cause another dispatch of new actions (commonly accompanied with api calls, which makes things even worse, because that would be an external side effect). I call them "unstable states" because when you reach that state, the app causes something that makes it reach another, now "stable" state, by dispatching new actions.

A common case is to have a component that when mounted, or receiving different props, dispatch an async action that fetches some data from an api and dispatches multiple actions to update the state in the different points where the api call starts, succeeds, or fails.

This, while completely understandable, is not expected when using the devtools, at least when doing stuff like time travelling, or disabling/enabling particular actions.

I've been trying to think about a possible solution to this, but it seems quite tricky.
An idea would be to disable nested dispatches when replaying actions, another one to have a way to detect the actions that triggered new actions (we could know that if they dispatch in the same tick) and show them with a warning in the DebugPanel or something.

Or we could just add a note in the readme saying that this case is treated as a side-effectful render, and the devtools need renders have no side effects other than updating the DOM in order to work properly.

Anyway, I think there should be a better solution to having to dispatch actions when mounting or updating props, but when using async actions, I think this is the only way to get the data needed for that component without coupling everything to the original action creator.

Does this make sense at all? Let me know if this is unclear, I may have explained this poorly

@gaearon
Copy link
Contributor

gaearon commented Oct 22, 2015

Shouldn't action creators have smart checks that prevent them from firing when data is available, anyway? Like in examples/real-world.

@leoasis
Copy link
Author

leoasis commented Oct 23, 2015

I don't see how that helps when replaying actions with devtools (though I agree you should have those checks).

In fact, I've reproduced the same issue in the real world example, see the following recording:

devtools

I'm searching for a user, then cancel all the actions up to the route change. If I replay that action, the hook fires in the component and triggers new actions, which appear after the cancelled ones. This makes the replaying cause side effects and alter the timeline of actions we already have, which is something unexpected.

We should see if any of the approaches I proposed in the comment before could solve this, or if anyone has a better idea to tackle this. Or at least show a note in the readme next to the one about the reducers being pure. I'd really would like to have the replaying working even in this scenario, so there may be a way to solve this.

@gaearon
Copy link
Contributor

gaearon commented Oct 23, 2015

Oh, I get it now.

The solution is simple: the dev tools ideally should include a UI for entering a “replay mode”. Right now it's implicitly “on” all the time but it's just because I didn't have time to work on the UX.

The workflow I envision is that you'd choose a few specific actions you want to play with, and then you can toggle them. However, in this mode, all new actions will be ignored. This is what you want in the replay mode: ignore any new action.

We just don't have a UI for that.

@leoasis
Copy link
Author

leoasis commented Oct 23, 2015

And what would happen with the potential api calls and other side effects in the action creators that are fired there, firing thunk actions which the devtools don't see? (I'm not sure about this part, I'm assuming the devtools only work with the final actions, so thunks happen outside of the devtools world) It would be nice to avoid firing them, especially if they mutate something outside.

@gaearon
Copy link
Contributor

gaearon commented Oct 23, 2015

They would happen. But they wouldn't change the state of Redux app, so I'd say these would just be wasted GET calls. At this point I guess it's up to you to have a mechanism to block the API calls in some cases. Since Redux doesn't have a first class notion of effect like Elm, there's nothing we can do here.

@jdeal
Copy link

jdeal commented Mar 5, 2016

@leoasis @gaearon I think preventing API calls and other side effects just requires following what's probably a better pattern in action creators anyway: don't produce side effects there. If you do that, you just need a "null middleware" at the front of your middleware stack that you can use to prevent side effects during replay.

I think the real-world example uses only thunks and declarative actions, so you could try this today with a global toggle and a middleware like this:

export default () => next => action => {
  if (!IS_DEVTOOLS_REPLAYING) {
    return next(action);
  }
};

Then just use the console as your toggle UI. :-)

For anybody following along at home, by declarative actions, I mean:

function fetchUser(login) {
  return {
    [CALL_API]: {
      types: [ USER_REQUEST, USER_SUCCESS, USER_FAILURE ],
      endpoint: `users/${login}`,
      schema: Schemas.USER
    }
  }
}

Rather than something like:

function fetchUser(login) {
  return {
    [CALL_API]: {
      types: [ USER_REQUEST, USER_SUCCESS, USER_FAILURE ],
      promise: fetch(`users/${login}`),
      schema: Schemas.USER
    }
  }
}

Those look deceptively similar, but they're very different. In the latter, the work has already begun, and there's no way to shut it off. But the former is just a description of the work to be done, so our null middleware can intercept and cancel it.

@leoasis
Copy link
Author

leoasis commented Mar 7, 2016

@jdeal yeah, completely agree. Avoiding side effects in the action creators has been something I've been doing for some time now. The fact that you just declare what you want to happen and move that logic to the middleware (or even a store subscription is enough sometimes) gives you a lot more advantages. In this particular case allows to be able to avoid this problem.

Still wrapping my head around what's the best approach out there (or even not yet discovered), sagas, effects in the reducers (like in Elm), effects in the action creators, or something else. All have their tradeoffs, so not sure yet.

But to come back to the point being discussed here, yeah, you need to solve this in userland because as @gaearon said, the notion of effect is not baked in Redux nor in the devtools, so it's not something that can be solved directly in the library.

@nwwells
Copy link

nwwells commented Mar 10, 2016

I just googled into this. I've got a store subscriber that performs side effects. I'm perfectly satisfied preventing those side effects from executing in my code, but I need some way to know whether the state change I'm getting a callback for was caused by a replay or an actual action execution. Is such a mechanism available? Not finding it anywhere if it is.

@zalmoxisus
Copy link
Collaborator

Just released v2.3.0 of the extension, where we expose methods to deal with it. See the description and the example of usage for more details.

@exciton
Copy link

exciton commented Sep 24, 2016

It's pretty simple to write a middleware which will simply ignore all dispatched actions if devtools is time travelling as such:

let isMonitorAction
let isTimeTraveling

const middlewares = []
if (__DEV__) {
  //Basic middleware which will not dispatch actions when we're using devtools
  middlewares.push( () => next => action => { return isMonitorAction() || isTimeTraveling() ? action : next(action)})
}

const composedStore = compose(
  applyMiddleware(...middlewares),
  __DEV__ && typeof window === 'object' && typeof window.devToolsExtension !== 'undefined' ?
    window.devToolsExtension({
      getMonitor: (monitor) => { isMonitorAction = monitor.isMonitorAction; isTimeTraveling = monitor.isTimeTraveling }
    }) :
    f => f
)

export default store = composedStore(createStore)(reducer)

@zalmoxisus
Copy link
Collaborator

zalmoxisus commented Oct 4, 2016

Now you can lock changes and avoid side effects, see the post for details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants