Use Promise inside a function that return new Promise

Question

I know about anti-pattern that can be happened while using Promises in javascript. This is my scenario. I have a request function that return a Promise. I want to use this inside another function and execute some code in then and then resolve it or reject it in some condition:

export const login = (req: IRegisterAuthReq): Promise<IUserTokenResponse> => {
  return new Promise((resolve, reject) => {
    AuthApi.login(req)
      .then(response => {
        if (response.success) {
          store.dispatch(setUserLoggedIn(true));
          resolve(response.data as IUserTokenResponse);
        } else {
          reject();
        }
      })
      .catch(reject)
      .finally(() => {
        store.dispatch(setAppLoading(false));
      });
  });
};

This is my code, I call AuthApi.login function and execute some code and then resolve value.

and then use it like this:

login(req)
  .then(authoredSuccessful)
  .catch(authoredUnSuccessful);

I want to know if this approach is anti-pattern or bad, and if it is how can I achieve this functionality in best way.

"*I want to know if this approach is anti-pattern*" [yes, it is](https://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it). There is almost no reason to use the `Promise` constructor. It's only useful if you *don't* already have a promise. If you use `.then` already, it's a promise. You're just adding a promise around your promise. — VLAZ, Feb 15 '21 at 20:57

jfriend00 · Answer 1 · 2021-02-15T23:23:56.830

I want to know if this approach is anti-pattern or bad, and if it is how can I achieve this functionality in best way.

Yes, wrapping an existing promise with another manually created promise is an anti-pattern. You're unnecessarily creating an extra promise and besides adding unnecessary code, it's also very easy to make mistakes in error handling. See Promise anti-patterns for more info on promise anti-patterns. This one is known as the "Construction Anti-Pattern". There are a number of other promise-related anti-patterns here.

Instead, you should be returning the promise you already have and using return values or exceptions inside the .then() handler to control the final resolved value of the promise chain.

export const login = (req: IRegisterAuthReq): Promise < IUserTokenResponse > => {
    return AuthApi.login(req).then(response => {
        if (response.success) {
            store.dispatch(setUserLoggedIn(true));
            return response.data;
        } else {
            // make promise chain reject
            throw new Error("login not successful");
        }
    }).finally(() => {
        store.dispatch(setAppLoading(false));
    });
};

score 1 · Accepted Answer · edited Feb 15 '21 at 21:02

In my opinion, it's prettier to just chain promise call :

export const login = (req: IRegisterAuthReq): Promise<IUserTokenResponse> => {
  return AuthApi.login(req)
      .then(response => {
        if (response.success) {
          store.dispatch(setUserLoggedIn(true));
          return response.data as IUserTokenResponse;
        }
        throw new Error("Auth error")
      })
      .finally(() => store.dispatch(setAppLoading(false)));
};

And using async :

export const login = async (req: IRegisterAuthReq): Promise<IUserTokenResponse> => {
  const response = await AuthApi.login(req);
  
  try {
    if (response.success) {
      store.dispatch(setUserLoggedIn(true));
      return response.data as IUserTokenResponse;
    }
  
    throw new Error("Auth error");
  } finally {
    store.dispatch(setAppLoading(false));
  }
};

Tnx, I will wait for other answers, If there is no better answer, I will accept yours :) — BeHappy, Feb 15 '21 at 21:03

score -1 · Answer 3 · answered Feb 15 '21 at 20:25

-1

You can do something like:

login()
.then( () => {
  handleSuccess()
})
.then( () => {
  doAnyOtherPromise()
})
.catch( error => {
  // here error can be an error rejected by any previous promise
  console.log(error)
});

More info on how to you promises here

answered Feb 15 '21 at 20:25

Maxime Chevallier

277
1
4
14

1

This doesn't answer the question. Author want's to know about better ways to implement login not how promise chaining works. – Lakshya Thakur Feb 15 '21 at 20:29

Use Promise inside a function that return new Promise

3 Answers3