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

Clarify terminology #237

Merged
merged 3 commits into from
Mar 13, 2019
Merged

Clarify terminology #237

merged 3 commits into from
Mar 13, 2019

Conversation

jmm
Copy link
Contributor

@jmm jmm commented Feb 8, 2019

Currently it describes the action creator as the thunk.

@timdorr
Copy link
Member

timdorr commented Feb 8, 2019

This is more correct as-is. Thunks can take arguments. That doesn't make them action creators; they still return a function instead of an object. Also, they're not just for async side effects.

@timdorr timdorr closed this Feb 8, 2019
@markerikson
Copy link
Contributor

markerikson commented Feb 8, 2019

FWIW, I think the first part of the comment more accurately reflects how I would describe them myself. Second comment seems off, though.

The function with the (dispatch, getState) => any signature is the actual thunk. The function that creates and returns that function is a "thunk action creator".

@jmm
Copy link
Contributor Author

jmm commented Feb 8, 2019

Hi @timdorr, are you calling the outer function, inner function, or both a thunk?

If the outer function in the code snippet I edited the comment for isn't an action creator, then what is a "thunk action creator"? And if the inner function isn't a thunk, then what is a "thunk async action"?

The terminology is muddled right now and the parts I proposed changing aren't even internally consistent with the rest of the docs and that interpretation is inconsistent with all of the sources you link to:

Redux Thunk middleware allows you to write action creators that return a function instead of an action

// Thunk middleware lets me dispatch thunk async actions
// as if they were actions!

store.dispatch(
  makeASandwichWithSecretSauce('Me')
);

The return value of makeASandwichWithSecretSauce is what's getting dispatched.

This directly refers to the inner function as the thunk:

// It even takes care to return the thunk’s return value
// from the dispatch, so I can chain Promises as long as I return them.

store.dispatch(
  makeASandwichWithSecretSauce('My partner')
).then(() => {
  console.log('Done!');
});

Redux documentation:

An async action is a value that is sent to a dispatching function [...] They are often asynchronous primitives, like a Promise or a thunk

Redux thunk docs:

A thunk is a function that returns a function.

What the heck is a 'thunk'?:

A thunk is [...] a function that’s returned by another. Like this:

function not_a_thunk() {
  // this one is a "thunk" because it defers work for later:
  return function() {
    console.log('do stuff now');
  };
}

The StackOverflow answers by you link to by @gaearon specifically describe the outer function in this scenario as an action creator.

http://stackoverflow.com/questions/35411423/how-to-dispatch-a-redux-action-with-a-timeout/35415559#35415559

However it lets us declare showNotificationWithTimeout() as a regular Redux action creator:

export function showNotificationWithTimeout(text) {
  return function (dispatch) {
    const id = nextNotificationId++
    dispatch(showNotification(id, text))

    setTimeout(() => {
      dispatch(hideNotification(id))
    }, 5000)
  }
}

http://stackoverflow.com/questions/34570758/why-do-we-need-middleware-for-async-flow-in-redux/34599594#34599594

But with Thunk Middleware you can write it like this:

// action creator
function loadData(userId) {
  return dispatch => fetch(`http://data.com/${userId}`)
    .then(res => res.json())
    .then(
      data => dispatch({ type: 'LOAD_DATA_SUCCESS', data }),
      err => dispatch({ type: 'LOAD_DATA_FAILURE', err })
    );
}

https://medium.com/fullstack-academy/thunks-in-redux-the-basics-85e538a3fe60

const thunkedLogin = () =>     // action creator, when invoked…
  () =>                        // …returns a thunk, which when invoked…
    axios.get('/api/auth/me')  // …performs the actual effect.
    .then(res => res.data)
    .then(user => {
      store.dispatch(simpleLogin(user))
    })

@markerikson
Copy link
Contributor

FWIW, I personally dislike the phrase "async action" and want to remove it in the upcoming docs revamp.

A thunk function isn't an "action". It's not a plain object, and it doesn't have a type field. It's a different kind of thing. The thunk middleware teaches dispatch how to accept things that aren't plain action objects. The phrase "async action" is confusing.

@jmm
Copy link
Contributor Author

jmm commented Mar 13, 2019

Thanks @markerikson.

Second comment seems off, though.

I want to come back to that after getting the thunk vs. action creator part straight and understand what seems off to you.

FWIW I don't think "async action" would really be problematic if all of the terminology was used consistently, e.g. "async action creator", "async action", "thunk action creator", "thunk". But what do you think would be a better general term for these dispatchable things that could be a thunk, promise, or ...?

So is someone going to reopen this since the content I edited is clearly inconsistent with other parts of your own docs and all of the sources you link to? The dismissive handling of this is disappointing.

@markerikson
Copy link
Contributor

I'm not sure there's a single specific term that would be used for "passing other things to dispatch".

Yeah, lemme tweak the wording on that second paragraph a bit, and then I'll merge this.

@markerikson markerikson reopened this Mar 13, 2019
@markerikson markerikson merged commit bacc013 into reduxjs:master Mar 13, 2019
@jmm
Copy link
Contributor Author

jmm commented Mar 14, 2019

Thanks @markerikson, I appreciate that 🙌

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

Successfully merging this pull request may close these issues.

None yet

3 participants