bulentv / js_zklib

Attendance Machine Library for NodeJS with a connection to the network using the UDP protocol and port 4370
GNU General Public License v2.0
56 stars 46 forks source link

Add support for Promises #17

Open mribichich opened 6 years ago

mribichich commented 6 years ago

What do you think about supporting Promises?

If you dont want to remove callbacks for backwards compatibility, there's a method that could be use to support both

Basically if the user doesn't provide a callback a Promise is return, that way the lib can support both

There's this library that implements this really easy, and with the advantage that's really easy to remove callbacks and this lib in the future if you want to.

The lib is nodeify

Simple sample:

var Promise = require('nodeify').Promise;

function myAsyncMethod(arg, callback) {
  return new Promise(function (resolver) {
    //do async work
  })
  .nodeify(callback);
}

So in the future if you want to remove callbacks, all you have to do is remove:

what do you think? Promises will make much more friendly and es2015, and allow the use of async/await

bulentv commented 6 years ago

Sure, why not have Promises too if this is the time to switch to es2015.

Feel free to start implementing it but as you pointed out, we need to continue supporting the old API. So I put some deprecation warnings for renamed methods so that they can stay till the v0.3

We can do the same on promises (if possible?)

mribichich commented 6 years ago

I'll wait until you finish the transformation to es2015. Or you 're not planning on doing it for now?

Since the change to es2015 or Promises is a huge one, merging it will be hard y we change them separately. I can help you with es2015 if you want.

mribichich commented 6 years ago

I see you already did the transformation. great!

bulentv commented 6 years ago

Yes I did but of course it needs to be tested on a real system. You can directly push your changes. We can test it little more before implementing Promises, maybe in a new branch first?

mribichich commented 6 years ago

I can test were I'm using it. I dont use every function but I could test them.

I will try to set a couple of unit tests for test the callback/promise setup

caobo171 commented 4 years ago

I Changed it in my repo to using asyn / await !!

Check my repo , I've already fixed many bugs : https://github.com/caobo171/node-zklib