0
\$\begingroup\$

Given this method, we don't know which syntax is the more appropriate.

async finishUpgrade({id}) {
    return new Promise((resolve, reject) => {
      let options = {
        method: 'POST',
        uri: this.apiUrl + `${id}/?action=proceed`,
        body: {}
        json: true
      };

      try {
        const res = await req(options);
        resolve(res);
      } catch(err) {
        reject(err);
      }
    });
  }

As you can see we use return new Promise(...).

Would it be correct/a better way to refactor it this way:

async finishUpgrade({id}) {
  let options = {
    method: 'POST',
    uri: this.apiUrl + `${id}/?action=proceed`,
    body: {}
    json: true
  };

  try {
    const res = await req(options);
    return Promise.resolve(res);
  } catch(err) {
    return Promise.reject(err);
  }
}
\$\endgroup\$
4
  • 1
    \$\begingroup\$ Please see What to do when someone answers. I have rolled back Rev 3 → 2. \$\endgroup\$ Sep 28 '17 at 23:23
  • \$\begingroup\$ You are right we'll ask another question. Our bad! \$\endgroup\$ Sep 28 '17 at 23:24
  • \$\begingroup\$ @200_success - I'm OK with the edit. I've updated my answer to include that case too. \$\endgroup\$
    – jfriend00
    Sep 28 '17 at 23:25
  • \$\begingroup\$ @jfriend00 In that case, go ahead and adjust both the question and answer. \$\endgroup\$ Sep 28 '17 at 23:28
4
\$\begingroup\$

Given this method, we don't know which syntax is the more appropriate.

Neither is appropriate as both your options employ promise anti-patterns (creating new promises when you can just work with and return the one you already have).

Assuming req() already returns a promise (which you imply with your use of await), you should just return that promise directly. I see no reason to even use async or await here.

// presumably from your use of this, this is a method declaration
function finishUpgrade({id}) {
  let options = {
    method: 'POST',
    uri: this.apiUrl + `${id}/?action=proceed`,
    body: {}
    json: true
  };
  return req(options);
}

this.finishUpgrade({id: someId}).then(result => {
    // upgrade done here
}).catch(err => {
    // error here
});

When you do need to modify the return value, you just add a .then() handler and the modified value you return from that .then() handler becomes the new resolved value:

// presumably from your use of this, this is a method declaration
function finishUpgrade({id}) {
  let options = {
    method: 'POST',
    uri: this.apiUrl + `${id}/?action=proceed`,
    body: {}
    json: true
  };
  return req(options).then(result => {
      // change the resolved value of the promise to our data object
      let data = { someData: someValue };
      data.response = result;
      return data;
  });
}

this.finishUpgrade({id: someId}).then(result => {
    // upgrade done here
}).catch(err => {
    // error here
});
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.