0

The code works but I'm wondering if I'm over promising.

I have this Redux Action

import Promise from 'bluebird';
const uploadAsynch = Promise.promisify(api.upload);

uploadFiles : function(data, dispatch){
    var data = {
      ep:"EP_UPLOAD",
      payload: {
       files: data.files,
       profile: data.profile
      }
    }
    uploadAsynch(data).then((result)=>{
      dispatch({type: FILES_UPLOADED})
    });
  },

api.upload is the following

import axios from 'axios';

upload : function(data, callback){
    var files = new FormData();
    for(var i=0; i<data.payload.files.length; i++){
      files.append('files', data.payload.files[i], data.payload.files[i].name);
    }
    axios.post(apiEndpoints[data.ep], files, {
      headers: {
        'accept': 'application/json',
        'Accept-Language': 'en-US,en;q=0.8',
        'Content-Type': `multipart/form-data; boundary=--*`,
      }
    })
    .then((response) => {
      callback(null, response)
    }).catch((error) => {
      callback(error)
    });
  },

So I'm wondering. If Axios is a promise based request client is it correct to wrap it with bluebird in the action?

Ando
  • 1,802
  • 4
  • 25
  • 47
  • What do you mean by *over promising*? Do you really need `upload` to be callback-based? – Ry- Jun 02 '17 at 04:20
  • That is what I wonder... Should it just be a return and then handle with a switch that response? – Ando Jun 02 '17 at 04:22
  • Not sure what you mean by “with a switch” either. If you don’t need your `api.upload` to be callback-based, though, it’s probably best to make it just return a promise, yes. – Ry- Jun 02 '17 at 04:23
  • So in the action I would just have `api.upload(data)` and then api.upload just returns true or error depending on what axios gets as response from the server? Where do I fire the `dispatch` from? From within the axios upload function? – Ando Jun 02 '17 at 04:27
  • `api.upload` just works exactly the same as `uploadAsynch`. – Ry- Jun 02 '17 at 04:31

1 Answers1

0

I'd say you're under-promising, because you don't use promises to their full capability. However, you do fall back to callbacks and at the same time promisify that function again, which is pretty pointless. Just return the promise you had in the first place.

// no promisification
uploadFiles: function(data, dispatch){
  var data = {
    ep:"EP_UPLOAD",
    payload: {
      files: data.files,
      profile: data.profile
    }
  }
  return api.upload(data).then(result =>
//^^^^^^
    dispatch({type: FILES_UPLOADED})
  );
}

import axios from 'axios';

upload: function(data) { // no callback
  var files = new FormData();
  for (var i=0; i<data.payload.files.length; i++){
    files.append('files', data.payload.files[i], data.payload.files[i].name);
  }
  return axios.post(apiEndpoints[data.ep], files, {
//^^^^^^
    headers: {
      'accept': 'application/json',
      'Accept-Language': 'en-US,en;q=0.8',
      'Content-Type': `multipart/form-data; boundary=--*`,
    }
  });
}
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • v interesting. I thought that a promise's chain is tied to it but from your example, which works, it seems like I can apply the chain one level above (from where i call the promise). Is that correct to say? Thanks for the answer. – Ando Jun 02 '17 at 04:39
  • I'm not sure what "a promise's chain" means to you. A chain always consists of *multiple* promises, and it can't be "tied" to something. How [`then` returns new promises](https://stackoverflow.com/a/22562045/1048572) is the real magic - and yes, promises are *values* that can be passed around and be `return`ed from functions like any other, it doesn't matter when/where/whether the `.then()` method is called on it. – Bergi Jun 02 '17 at 05:13