Misleading docs for Action Creators

[aikoven]

https://github.com/rackt/redux/blob/master/docs/basics/Actions.md
Docs for Action Creators state that they should be pure, which is presumably wrong.
Actually I see them as the only place in Redux data flow that should encapsulate all non-purity and side-effects.

[sompylasar]

👍 wrt redux-thunk and other side-effecty usecases (server requests etc.) that usually reside (and persumably should) in the "Action Creators".

[chivorotkiv]

In redux-thunk Action Creators are pure. But what they return may be not pure.

[aikoven]

Here's another example from docs: non-pure and not a thunk.

export function receivePosts(reddit, json) {
  return {
    type: RECEIVE_POSTS,
    reddit,
    posts: json.data.children.map(child => child.data),
    receivedAt: Date.now()
  }
}

https://github.com/rackt/redux/blob/master/docs/advanced/AsyncActions.md

[just-boris]

Yeah, here is better to get lastUpdate as latest date from the posts. It would make function pure and tolerant to unsynced clocks between client and server.

[aikoven]

Yeah but is there any motivation for action creators to be pure?
What they do is convert IO events to actions and are not used anywhere else in Redux pipeline.
I understand such a requirement for reducers as they are meant to yield same result for same sequence of actions, but action creators are not involved in that.

[just-boris]

ActionCreators description has a motivation to make them pure.

Redux action creators are pure functions with zero side-effects [...] This makes them more portable and easier to test.

So, if your issues require to use side-effects in action creators, you can do it, it will work with redux-devtools and others. But you will got more complicated code, which would be difficult to test. Since we are talking about receivePosts creator, it would be better to pass current timestamp as argument

function receivePosts(reddit, json, timestamp) {
  return {
    type: RECEIVE_POSTS,
    reddit,
    posts: json.data.children.map(child => child.data),
    receivedAt: timestamp
  }
}

Now you can use the function in test without any mocks

it("should update posts and save timestamp", function() {
  store.dispatch(receivePosts(testReddit, responseJson, 13456575));
  expect(store.getState().lastUpdated).toEqual(13456575)
});

[sompylasar]

If so, where in Redux architecture should we put the side-effects logic?

[timdorr]

@sompylasar That should be managed by middleware. The middleware can let the store know about the state of those side effects, but the actual side effects are best managed by a middleware. redux-promise is what I use, but redux-thunk is another way to go about it.

As a reminder, a Promise is just an object with no side effects until .then() is called on it. So, returning a Promise from an action creator still retains its purity.

[gaearon]

@timdorr FWIW the is no need for action creators to be pure.

[timdorr]

True. It doesn't need to be pure, but it certainly helps. Should we update the docs then?

[sompylasar]

@timdorr I'd avoid putting application logic into middleware, it's not for that.

A promise is an object that tells you the state of some operation that will end later, but that has already been started. You cannot guarantee that the side-effects only run on .then when you receive the promise in your code; someone might have called .then on it before.

[timdorr]

@sompylasar I'm not putting application logic in my middleware. I'm just saying the handling of side effects (normally Promises) is done by middleware.

And yes, you're right about the Promise definition. My bad on that.

[sompylasar]

@timdorr handling promises does not equal to handling side-effects because the side-effects ahs already happen before the promise was even created.

[timdorr]

@sompylasar A Promise is a representation of a kind of side effect. Yes, there are other types of side effects, so you would have middleware to handle those as well, where you can (Date.now or Math.random might be an exception).

In any case, that's kind of getting off topic. Should the docs on action creators be changed to loosen the language a bit? I think so, but pure function should still be mentioned to make sure people know their advantages.

[sompylasar]

@timdorr I think the docs should include both the simple case (pure action creator) and the complex case (action creator that triggers side-effects and dispatches several actions, effectively events).

[timdorr]

@sompylasar You cannot do the complex case you've mentioned without middleware. We already have this covered here: http://redux.js.org/docs/advanced/AsyncFlow.html

Actually, I'm going to reverse my position on this. I don't think it should be documented that action creators can be non-pure in that section of the documentation. We should be respectful of the audience reading these docs, who are very likely going to be beginners. They are still forming their coding habits, so getting them to produce simple, testable code is important. Especially in one of the first documents you'll read when learning about Redux. Technically, it's a lie, but it's a white lie with good intentions.

[sompylasar]

@timdorr Agreed.

[aikoven]

@timdorr But this white lie made me as a beginner get confused about Redux requirements. And it also contradicts the usage of Date.now() later on, which made me return to this part again to see if I'm missing something.

I agree that forming good habits is important, but forming wrong understanding of a subject is a bad way of doing it.

[gaearon]

That doc right now is wrong. It was written very early. Action creators don't need to be pure at all. Please send a PR to fix the doc.

On 4 Dec 2015, at 04:58, Daniel Lytkin notifications@github.com wrote:

@timdorr But this white lie made me as a beginner get confused about Redux requirements. And it also contradicts the usage of Date.now() later on, which made me return to this part again to see if I'm missing something.

I agree that forming good habits is important, but forming wrong understanding of a subject is a bad way of doing it.

—
Reply to this email directly or view it on GitHub.

[gaearon]

Closed via b0763b1.