Bug when API payload has the same structure as an action

[mrtnbroder]

Not sure if this is actually a bug but I just came across this really edge-case-y thing where my API payload had the same structure like an action, which caused me a 3h headache why my promise never reached the fulfilled part, even though the API call was successfull.

My Action looks like this:

export const createCar = (data) => ({
  type: types.CREATE_CAR,
  payload: {
    promise: API.createCar(data)
  }
})

API:

API.createCar = function createCar(data) {
    return fetch(`${API_URL}/cars`, {
      method: 'POST',
      body: JSON.stringify(data)
    }).then((r) => r.json())

Reducer:

  ...

  [`${types.CREATE_CAR}_PENDING`]: (state) => {
    return {
      ...state,
      isPending: true
    }
  },

  [`${types.CREATE_CAR}_FULFILLED`]: (state, { payload }) => {
    return {
      ...state,
      isPending: false,
      data: {
        ...state.data,
        [payload.id]: payload
      }
    }
  },
  ...

Now I was wondering why my action never reached the 'fulfilled' state. This was because my API payload looked like this:

{
  id: 12345
  type: 'car',
  payload: {
    foo: 'bar',
    ...etc.
  }
}

What now happened is that this payload got fired as an action and it never reached the fulfilled state because of that.

I wonder if there is some way around that? currently I'm wrapping the api response in another object so it reaches the fulfilled state but I feel like this should not actually happen. It should always reach one of my pending...fulfilled...rejected states.

[tomatau]

This is a tough one! We want to support being able to fire a new action meta in the fulfilled/rejected case and reuse the existing type.

The use case for this was one of my own - some middleware use data in the meta for performing side effects with the payload, and often you want these effects to only apply to rejected/fulfilled and not pending. So we support a new action dispatch when the resolved object has a meta or payload.

Right now to check if that fulfilled/rejected object is an action that gets dispatched, we check for either meta or payload properties... we could remove the check for a payload property and only look for meta. Or even make sure the resolved object has both meta and payload. I don't see a use-case for when the resolved object just has a payload property - it wouldn't give any benefit to dispatch it as an action in that case.

But we would still have the same problem if a resolved payload wanted to contain it's own 'meta' property.

@pburtchaell what do you think?

[pburtchaell]

@tomatau Perhaps we could make firing new action meta optional by moving it into the middleware configuration? This way @mrtnbroder could just disable that functionality and everything would work as expected.

[tomatau]

Yeh that would work! Not sure if it feels right though? Would be nice if the middleware had consistent behaviour.

I was thinking it might be ok to remove the functionality from the object resolved by a Promise -- as the thunk functionality can also achieve the same goal. It's a bit more verbose to use a thunk in the resolve, but less 'hidden'. Magically making a new action sometimes and just making a payload depending on what is resolved is starting to feel a bit wrong in general.

[pburtchaell]

Okay, so remove the functionality and release a new major version? I would be fine with that since I haven't actually used this functionality yet. Do you want to make a PR?

[tomatau]

I'm super busy at the moment so don't think I'll be able to! Although it is literally just deleting code and tests :)

[pburtchaell]

No worries, just seeing if you could. I'll be able to take care of this
over the weekend.
On Feb 23, 2016 03:19, "Thomas" notifications@github.com wrote:

I'm super busy at the moment so don't think I'll be able to! Although it
is literally just deleting code and tests :)

—
Reply to this email directly or view it on GitHub
#61 (comment)
.

[mrtnbroder]

Any update on this? 👯

[pburtchaell]

I am working on it! I have a branch I started locally for a version 3.0.0 release and this will be included in it. I hope to finish it up soon.

[Cinamonas]

Just stumbled upon this. The API I am consuming is responding with objects that have meta key.

[pburtchaell]

This is fixed in the new API release: redux-promise-middleware@3.0.0. Please let me know if you have any issues.