src,test,build: allow NAPI_VERSION env var and templatize AttachData …

…callback PR-URL: #1399 Reviewed-By: Chengzhong Wu <[email protected]> Signed-off-by: Gabriel Schulhof <[email protected]>
nodejs · Nov 5, 2023 · c52e764 · c52e764
1 parent ab14347
commit c52e764
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 20 deletions.
diff --git a/common.gypi b/common.gypi
@@ -1,6 +1,6 @@
 {
   'variables': {
-    'NAPI_VERSION%': "<!(node -p \"process.versions.napi\")",
+    'NAPI_VERSION%': "<!(node -p \"process.env.NAPI_VERSION || process.versions.napi\")",
     'disable_deprecated': "<!(node -p \"process.env['npm_config_disable_deprecated']\")"
   },
   'conditions': [

diff --git a/napi-inl.h b/napi-inl.h
@@ -31,22 +31,22 @@ namespace details {
 // Node.js releases. Only necessary when they are used in napi.h and napi-inl.h.
 constexpr int napi_no_external_buffers_allowed = 22;
 
+template <typename FreeType>
+inline void default_finalizer(napi_env /*env*/, void* data, void* /*hint*/) {
+  delete static_cast<FreeType*>(data);
+}
+
 // Attach a data item to an object and delete it when the object gets
 // garbage-collected.
 // TODO: Replace this code with `napi_add_finalizer()` whenever it becomes
 // available on all supported versions of Node.js.
-template <typename FreeType>
+template <typename FreeType,
+          napi_finalize finalizer = default_finalizer<FreeType>>
 inline napi_status AttachData(napi_env env,
                               napi_value obj,
                               FreeType* data,
-                              napi_finalize finalizer = nullptr,
                               void* hint = nullptr) {
   napi_status status;
-  if (finalizer == nullptr) {
-    finalizer = [](napi_env /*env*/, void* data, void* /*hint*/) {
-      delete static_cast<FreeType*>(data);
-    };
-  }
 #if (NAPI_VERSION < 5)
   napi_value symbol, external;
   status = napi_create_symbol(env, nullptr, &symbol);
@@ -1636,11 +1636,8 @@ inline void Object::AddFinalizer(Finalizer finalizeCallback, T* data) const {
       new details::FinalizeData<T, Finalizer>(
           {std::move(finalizeCallback), nullptr});
   napi_status status =
-      details::AttachData(_env,
-                          *this,
-                          data,
-                          details::FinalizeData<T, Finalizer>::Wrapper,
-                          finalizeData);
+      details::AttachData<T, details::FinalizeData<T, Finalizer>::Wrapper>(
+          _env, *this, data, finalizeData);
   if (status != napi_ok) {
     delete finalizeData;
     NAPI_THROW_IF_FAILED_VOID(_env, status);
@@ -1654,12 +1651,9 @@ inline void Object::AddFinalizer(Finalizer finalizeCallback,
   details::FinalizeData<T, Finalizer, Hint>* finalizeData =
       new details::FinalizeData<T, Finalizer, Hint>(
           {std::move(finalizeCallback), finalizeHint});
-  napi_status status = details::AttachData(
-      _env,
-      *this,
-      data,
-      details::FinalizeData<T, Finalizer, Hint>::WrapperWithHint,
-      finalizeData);
+  napi_status status = details::
+      AttachData<T, details::FinalizeData<T, Finalizer, Hint>::WrapperWithHint>(
+          _env, *this, data, finalizeData);
   if (status != napi_ok) {
     delete finalizeData;
     NAPI_THROW_IF_FAILED_VOID(_env, status);

diff --git a/test/common/index.js b/test/common/index.js
@@ -148,7 +148,7 @@ async function whichBuildType () {
       if (await checkBuildType(envBuildType)) {
         buildType = envBuildType;
       } else {
-        throw new Error(`The ${envBuildType} build doesn't exists.`);
+        throw new Error(`The ${envBuildType} build doesn't exist.`);
       }
     } else {
       throw new Error('Invalid value for NODE_API_BUILD_CONFIG environment variable. It should be set to Release or Debug.');