diegohaz / redux-saga-thunk

Dispatching an action handled by redux-saga returns promise
MIT License
221 stars 17 forks source link

Can't pass `thunk` to `_extend` #27

Open pi-chi opened 6 years ago

pi-chi commented 6 years ago

Hey there:

Apologies if this is way out of scope for your module. My environment is pretty much:

react-native-cli: 2.0.1 react-native: 0.55.1 ignite: 2.1.0 (w/ ignite-ir-boilerplate-andross) redux-saga: ^0.16.0 redux-saga-thunk: 0.7.1 reduxsauce: 0.7.0 mac os: 10.13.5

Maybe I'm doing something wrong here, or maybe no one else has tried, but not sure how/if I'm the only one who has faced this issue..

I've got a component with the following method (set up this way since the component is one of many in a list, and want to keep track of requests per item in list):

  async onPress () {
    this.setState({
      fetching: true
    })
    await this.props.deleteTodosRequest(this.props.todo)
    this.setState({
      fetching: false
    })
  }

And the basic map helper:

const mapDispatchToProps = (dispatch) => {
  return {
    deleteTodosRequest: (todo) => dispatch(DeleteTodosActions.deleteTodosRequest(todo))
  }
}

I've written my action creators like so using reduxsauce:

const { Types, Creators } = createActions({
  deleteTodosRequest: (data) => ({
    type: 'DELETE_TODOS_REQUEST',
    data,
    meta: {
      thunk: true
    }
  }),
  deleteTodosSuccess: ['payload'],
  deleteTodosFailure: null
})

And my saga:

export function * deleteTodos (api, action) {
  const { data } = action
  // get current data from Store
  // const currentData = yield select(DeleteTodosSelectors.getData)
  // make the call to the api
  const response = yield call(api.deleteTodos, data)

  // success?
  if (response.ok) {
    // You might need to change the response here - do this with a 'transform',
    // located in ../Transforms/. Otherwise, just pass the data back from the api.
    yield put(DeleteTodosActions.deleteTodosSuccess(response.data))
    yield put(TodosActions.removeTodos(data))
  } else {
    yield put(DeleteTodosActions.deleteTodosFailure())
  }
}

What I'm seeing here is that the following method from utils.js on line 66:

var generateThunk = exports.generateThunk = function generateThunk(action) {
  var thunk = getThunkMeta(action);

  return hasKey(action) ? _extends({}, thunk, {
    type: 'RESPONSE'
  }) : _extends({}, thunk || {}, {
    name: getThunkName(action),
    key: Math.random().toFixed(16).substring(2),
    type: 'REQUEST'
  });
};

tries to pass a truthy value to the _extend() method which throws the following error:

TypeError: In this environment the sources for assign MUST be an object.

I was able to solve this one of three ways. One by returning just the meta object in the getThunkMeta() method:

var getThunkMeta = exports.getThunkMeta = function getThunkMeta(action) {
  if (isThunkAction(action)) {
    return action.meta;
  }
  return null;
};

Two by wrapping the thunk in an object:

var generateThunk = exports.generateThunk = function generateThunk(action) {
  var thunk = getThunkMeta(action);

  return hasKey(action) ? _extends({}, { thunk: thunk }, {
    type: 'RESPONSE'
  }) : _extends({}, { thunk: thunk } || {}, {
    name: getThunkName(action),
    key: Math.random().toFixed(16).substring(2),
    type: 'REQUEST'
  });
};

Or three by adding an {} instead of a true to the action creator meta:

const { Types, Creators } = createActions({
  deleteTodosRequest: (data) => ({
    type: 'DELETE_TODOS_REQUEST',
    data,
    meta: {
      thunk: {}
    }
  }),
  deleteTodosSuccess: ['payload'],
  deleteTodosFailure: null
})

This wasn't tested against anything else tho, so not sure of implications.. If feasible I'm happy to submit a PR.

diegohaz commented 6 years ago

I've never seen this error before. I'm gonna check this out later today.

Feel free to start a PR with your changes. :)

Thanks for reporting.