1

I am wanting to refactor a Promise chain by extracting out some functions. Currently I have 

const getData = (uuid) => {
  return new Promise((resolve) => {
    fetch(
      // go fetch stuff
    )
    .then((response) => {
      if (!response.ok) {
        return resolve(false);
      }
      return response;
    })
    .then(fetchres.json)
    .then(response => {
      // Do more stuff that requires resolves that I will also want to refactor
    })
    .catch(err => {
      console.log(err);
      resolve(false);
    });
  });
};

So I want to extract the part where I resolve the unsuccessful responses. But pass along any successful ones. I have pulled it out like so. 

const resolveUnsuccessfulResponses = (response) => {
  if (!response.ok) {
    return response.resolve(false);
  }
  return response;
}


const getData = (uuid) => {
  return new Promise((resolve) => {
    fetch(
      // go fetch stuff
    )
    .then(resolveUnsuccessfulResponses)
    .then(fetchres.json)
    .then(response => {
      // Do more stuff that requires resolves that I will also want to refactor
    })
    .catch(err => {
      console.log(err);
      resolve(false);
    });
  });
};

Now I'm understandably getting the error resolve is not defined. How can I resolve this Promise in an external function? Should I pass resolve to my extracted function? That would seem clunky.

.then(response => resolveUnsuccessfulResponses(response, resolve))

I might end up having something like

.then(fetchres.json)
.then(parseResponseData)
.then(postDataSomewhere)
.then(doOtherThings)
.then(doEvenMoreCoolThings)

And to have to pass response and resolve to each of them seems wrong

Simon Legg
  • 683
  • 7
  • 20
  • If you think that `return resolve(false);` would [break the chain](http://stackoverflow.com/q/21576862/1048572) you are mistaken anyway. And you should avoid the [`Promise` constructor antipattern](http://stackoverflow.com/q/23803743/1048572)! – Bergi Jun 23 '16 at 18:09
  • @Bergi Thanks for that link on the Promise constructor antipattern! Also yes I've just realised that even currently that is just passing `false` on to `fetchres.json` – Simon Legg Jun 23 '16 at 18:30

1 Answers1

2

You should return a new Promise from your external functions aswell:

const resolveUnsuccessfulResponses = (response) => {
  return new Promise((resolve, reject) => {
    if (!response.ok) {
      return resolve(false);
    }
    return resolve(response);
  });
}
weisk
  • 2,468
  • 1
  • 18
  • 18
  • Ohhh right, ok thanks. I was thinking for the times I want to carry on passing the response down the chain it would have to be the same Promise filtering down. I guess that's not the case? – Simon Legg Jun 23 '16 at 18:06
  • Promises can only resolve to one value, so if you want to propagate the response down the chain, either you resolve with the response, or you resolve an extension of it: `resolve({ ...response, extraStuff })` – weisk Jun 23 '16 at 18:11
  • Also, wouldn't this cause an issue where when I `resolve(false)` this would now carry on down the chain to the next function. I don't want this to happen. In these cases I want to end it there with the `false` and skip the rest. – Simon Legg Jun 23 '16 at 18:12
  • Yes, I thought you explicitly wanted to continue even if the response is not correct. In the case that you want to stop it there, instead of `resolve`, do a `reject`! – weisk Jun 23 '16 at 18:13
  • Oh ok, no I would want to stop it there. As I understand it though a `reject` sends the Promise down to the `.catch`? Would this mean if I wanted to stop things throughout the chain for different reasons I would have to then deal with any 'error' cases in the catch block? – Simon Legg Jun 23 '16 at 18:20
  • Exactly, that's the 'correct' way of dealing with errors through promise chains. You can build your custom error on the function that performs the rejection though. – weisk Jun 23 '16 at 18:21
  • care to mark as correct answer if it helped you? :) – weisk Jun 23 '16 at 18:36