ThisIsRudigo / firebaseauth

Firebase authentication library - a node js wrapper around the Firebase REST API
42 stars 8 forks source link

Make exposed methods support promises and callbacks #16

Closed MrStLouis closed 6 years ago

MrStLouis commented 6 years ago

This is a simple way to update the exposed functions to support either a promise or a callback. It is a work in progress as I have only tested the registerWithEmail and signInWithEmail functions

I made each function return a promise and added or used the callback check block

if (typeof(callback) !== 'function'){
  callback = (err: any, res: any) => {
    if(err) return reject(err);
    resolve(res);
  }
}

the only problem I see would be in this area where the callback can return an error and a result

function completeRegistration(apiKey: string, name: string, photoUrl: string, authResult: any, callback: Function) {
  if (name || photoUrl) {
    // save name as well before returning to caller
    updateProfile(apiKey, authResult.token, name, photoUrl, (err: any, user: any) => {
      authResult.user = user;
      callback(err, authResult); // will return error as well if the profile update failed
    });
  }
  else {
    // no extra info to update
    callback(null, authResult);
  }
}

the last step was returning the functions inside the class methods in the index file so typescript sees the class methods as returning promises.

Like I said, this is a WIP and without unit testing set up I can't yet say for sure if everything will behave as expected. I also don't mind attempting another PR to setup testing for this repo as I need the practice and would love to contribute to this project.

itswisdomagain commented 6 years ago

Nice work man. Sorry for reverting late.

I want to make the following recommendations for making the methods return Promise if there is no callback provided. Key word is IF no callback is provided. It shouldn't return a promise if a callback is provided. Your current code returns promise always.

The following is a snippet of how returning promise selectively could be achieved, using the email signIn method as an example:

export function signIn(apiKey: string, email: string, password: string): Promise<FirebaseResponse>;
export function signIn(apiKey: string, email: string, password: string, callback: Function): void;
export function signIn(apiKey: string, email: string, password: string, callback?: Function): Promise<FirebaseResponse> | void {
    let promise;
    if (!callback) {
        promise = new Promise<FirebaseResponse>((resolve, reject) => {
            callback = (error: any, result: any) => error ? reject(error) : resolve(result);
        });
    }

    // normal code

    return promise;
}

If you get the drift, please make the adjustment and I'll accept the PR.