pokusew / node-pcsclite

Bindings over pcsclite to access Smart Cards
ISC License
60 stars 55 forks source link

remove deprecated v8::Handle to upgrade to node@12 #23

Closed zy4 closed 4 years ago

zy4 commented 5 years ago

the change is also needed for using electron 5.x: https://electronjs.org/blog/nodejs-native-addons-and-electron-5

jimmythesaint82 commented 5 years ago

It seems to compile mostly, just hitting these 2 errors.

../src/pcsclite.cpp:19:40: error: too few arguments to function call, single argument 'context' was not specified constructor.Reset(tpl->GetFunction());

../src/pcsclite.cpp:20:73: error: too few arguments to function call, single argument 'context' was not specified target->Set(Nan::New("PCSCLite").ToLocalChecked(), tpl->GetFunction());

zy4 commented 5 years ago

Getting the same, see: https://github.com/nodejs/node-addon-examples/issues/94 Maybe switching to N-API is the best here.

jimmythesaint82 commented 5 years ago

A few changes needed for new compatibility, Mostly Syntax ->

First All Handles to Local, done in this pull

GetFunction has changed

constructor.Reset(tpl->GetFunction()); target->Set(Nan::New("PCSCLite").ToLocalChecked(), tpl->GetFunction());

-> context / ToLocalChecked is now needed for GetFunction

Local context = Nan::GetCurrentContext(); constructor.Reset(tpl->GetFunction(context).ToLocalChecked()); target->Set(Nan::New("PCSCLite").ToLocalChecked(), tpl->GetFunction(context).ToLocalChecked());

All occurences of ToString() and UTF8Value need to be changed

v8::String::Utf8Value reader_name(info[0]->ToString());

->

Nan::Utf8String reader_name(Nan::To(info[0]).ToLocalChecked());

UInt32

ci->share_mode = info[0]->Uint32Value(); ci->pref_protocol = info[1]->Uint32Value();

->

ci->share_mode = info[0]->Uint32Value(context).FromJust(); ci->pref_protocol = info[1]->Uint32Value(context).FromJust();

ToObject

Local buffer_data = info[0]->ToObject();

->

Local buffer_data = Nan::To(info[0]).ToLocalChecked();

Think thats it

zy4 commented 5 years ago

Thanks! Have you tested it? Still getting errors.

jimmythesaint82 commented 5 years ago

Yeah, which errors? maybe i missed one...

zy4 commented 5 years ago

Updated the PR, getting

../src/cardreader.cpp:219:45: error: too few arguments to function call, single argument 'context' was not specified
    uint32_t out_len = info[1]->Uint32Value();
                       ~~~~~~~~~~~~~~~~~~~~ ^
/Users/namzin/.node-gyp/12.1.0/include/node/v8.h:2567:3: note: 'Uint32Value' declared here
  V8_WARN_UNUSED_RESULT Maybe<uint32_t> Uint32Value(
  ^
../.node-gyp/12.1.0/include/node/v8config.h:347:31: note: expanded from macro 'V8_WARN_UNUSED_RESULT'
#define V8_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
                              ^
../src/cardreader.cpp:220:46: error: too few arguments to function call, single argument 'context' was not specified
    uint32_t protocol = info[2]->Uint32Value();
                        ~~~~~~~~~~~~~~~~~~~~ ^
../.node-gyp/12.1.0/include/node/v8.h:2567:3: note: 'Uint32Value' declared here
  V8_WARN_UNUSED_RESULT Maybe<uint32_t> Uint32Value(
  ^
../.node-gyp/12.1.0/include/node/v8config.h:347:31: note: expanded from macro 'V8_WARN_UNUSED_RESULT'
#define V8_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
                              ^
../src/cardreader.cpp:273:37: error: no matching member function for call to 'ToObject'
    Local<Object> in_buf = info[0]->ToObject();
                           ~~~~~~~~~^~~~~~~~
../.node-gyp/12.1.0/include/node/v8.h:2532:44: note: candidate function not viable: requires single argument 'context',
      but no arguments were provided
  V8_WARN_UNUSED_RESULT MaybeLocal<Object> ToObject(
                                           ^
../.node-gyp/12.1.0/include/node/v8.h:2546:35: note: candidate function not viable: requires single argument 'isolate',
      but no arguments were provided
                    Local<Object> ToObject(Isolate* isolate) const);
                                  ^
../src/cardreader.cpp:274:47: error: too few arguments to function call, single argument 'context' was not specified
    DWORD control_code = info[1]->Uint32Value();
                         ~~~~~~~~~~~~~~~~~~~~ ^
../.node-gyp/12.1.0/include/node/v8.h:2567:3: note: 'Uint32Value' declared here
  V8_WARN_UNUSED_RESULT Maybe<uint32_t> Uint32Value(
  ^
../.node-gyp/12.1.0/include/node/v8config.h:347:31: note: expanded from macro 'V8_WARN_UNUSED_RESULT'
#define V8_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
                              ^
../src/cardreader.cpp:275:38: error: no matching member function for call to 'ToObject'
    Local<Object> out_buf = info[2]->ToObject();
                            ~~~~~~~~~^~~~~~~~
jimmythesaint82 commented 5 years ago

uint32_t out_len = info[1]->Uint32Value(); -> uint32_t out_len = info[1]->Uint32Value(context).FromJust();

uint32_t protocol = info[2]->Uint32Value(); -> uint32_t protocol = info[2]->Uint32Value(context).FromJust();

Local in_buf = info[0]->ToObject(); -> Local in_buf = Nan::To(info[0]).ToLocalChecked();

DWORD control_code = info[1]->Uint32Value(); -> DWORD control_code = info[1]->Uint32Value(context).FromJust();

Local out_buf = info[2]->ToObject(); -> Local out_buf = Nan::To(info[2]).ToLocalChecked();

If you need context . exaxmples 1,2,4 Local context = Nan::GetCurrentContext();

needs to be a the top of the function...

Give that a go.

pokusew commented 4 years ago

Hi @zy4,

Thank you very much for your PR. In the end I merged the #26 from @jimmythesaint82 as it was more complete and up-to-date. And now there is #27 that addresses the remaining issues.

Nevertheless, I really appreciate the work you did on this. 👍 Thanks again.