remove NAPI_EXPERIMENTAL

[acheronfail]

As discussed in #380 , I believe the NAPI_EXPERIMENTAL flag is causing build issues on supported node versions for this package.

I've looked through the git blame, and it appears that NAPI_EXPERIMENTAL was added a number of years ago for BigInt support. BigInt support was added to node in version 10, and this package says it supports 12, 14, 16 and 18.

So I think it's safe to remove NAPI_EXPERIMENTAL which will fix some failures on node version 18.20+ and other things.

Fixes #380

[markdirish]

Hi @acheronfail ,

Thanks for the PR! You are correct that NAPI_EXPERIMENTAL was added as a define when BigInt wasn't part of the main Node-API spec.

I am a little confused as to why NAPI_EXPERIMENTAL is causing an issue though. What I suspect is there is an interplay between the version of the node-addon-api package, the NAPI build version, and the version of Node.js. Can you try the following for me?

1. Update the version of node-addon-api to ^8.0.0 in the package.json. Currently it is some ancient version of the package, and I suspect it doesnt have the right napi-inl.h file for the right version of Node.js
2. In the napi_versions array at the bottom of the package.json, update the array to only include 8, 9. I think those versions should encapsulate all currently supported versions of Node.js: https://nodejs.org/api/n-api.html#node-api-version-matrix

If you make those changes, redownload the dependencies, and then rerun the build, does that fix the issue?

[acheronfail]

With this as the diff:

diff --git a/package.json b/package.json
index 2a6374a..359f6d1 100644
--- a/package.json
+++ b/package.json
@@ -41,7 +41,7 @@
   "dependencies": {
     "@mapbox/node-pre-gyp": "^1.0.5",
     "async": "^3.0.1",
-    "node-addon-api": "^3.0.2"
+    "node-addon-api": "^8.0.0"
   },
   "gypfile": true,
   "devDependencies": {
@@ -57,10 +57,8 @@
     "host": "https://github.com/markdirish/node-odbc/releases/download/v{version}",
     "package_name": "{name}-v{version}-{platform}-{arch}-napi-v{napi_build_version}.tar.gz",
     "napi_versions": [
-      3,
-      4,
-      5,
-      6
+      8,
+      9
     ]
   }
 }

I get this error:

/Users/acheronfail/src/node-odbc/node_modules/node-addon-api/napi-inl.h:68:12: error: no matching function for call to 'napi_add_finalizer'
  status = napi_add_finalizer(env, obj, data, finalizer, hint, nullptr);
           ^~~~~~~~~~~~~~~~~~
/Users/acheronfail/src/node-odbc/node_modules/node-addon-api/napi-inl.h:2379:29: note: in instantiation of function template specialization 'Napi::details::AttachData<Napi::details::CallbackData<Napi::Value (*)(const Napi::CallbackInfo &), Napi::Value>, &Napi::details::default_finalizer>' requested here
    status = Napi::details::AttachData(env, *result, data);
                            ^
/Users/acheronfail/src/node-odbc/node_modules/node-addon-api/napi-inl.h:2436:7: note: in instantiation of function template specialization 'Napi::CreateFunction<Napi::details::CallbackData<Napi::Value (*)(const Napi::CallbackInfo &), Napi::Value>>' requested here
      CreateFunction(env, utf8name, CbData::Wrapper, callbackData, &value);
      ^
../src/odbc.cpp:148:42: note: in instantiation of function template specialization 'Napi::Function::New<Napi::Value (*)(const Napi::CallbackInfo &)>' requested here
  exports.Set("connect", Napi::Function::New(env, ODBC::Connect));
                                         ^
/Users/acheronfail/Library/Caches/node-gyp/18.20.2/include/node/js_native_api.h:513:1: note: candidate function not viable: no known conversion from 'void (*)(napi_env, void *, void *)' (aka 'void (*)(napi_env__ *, void *, void *)') to 'node_api_nogc_finalize' (aka 'void (*)(const napi_env__ *, void *, void *)') for 4th argument
napi_add_finalizer(napi_env env,
^
1 warning and 1 error generated.

It's a slightly different type error, but still to do with napi_add_finalizer...

(Additionally, if I then proceed to remove NAPI_EXPERIMENTAL from binding.gyp it builds successfully again...)

[KerimG]

Great work, @acheronfail !

Any ETA on when this will be merged?