Skip to content

Commit c52e764

Browse files
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]>
1 parent ab14347 commit c52e764

File tree

3 files changed

+14
-20
lines changed

3 files changed

+14
-20
lines changed

common.gypi

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
'variables': {
3-
'NAPI_VERSION%': "<!(node -p \"process.versions.napi\")",
3+
'NAPI_VERSION%': "<!(node -p \"process.env.NAPI_VERSION || process.versions.napi\")",
44
'disable_deprecated': "<!(node -p \"process.env['npm_config_disable_deprecated']\")"
55
},
66
'conditions': [

napi-inl.h

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -31,22 +31,22 @@ namespace details {
3131
// Node.js releases. Only necessary when they are used in napi.h and napi-inl.h.
3232
constexpr int napi_no_external_buffers_allowed = 22;
3333

34+
template <typename FreeType>
35+
inline void default_finalizer(napi_env /*env*/, void* data, void* /*hint*/) {
36+
delete static_cast<FreeType*>(data);
37+
}
38+
3439
// Attach a data item to an object and delete it when the object gets
3540
// garbage-collected.
3641
// TODO: Replace this code with `napi_add_finalizer()` whenever it becomes
3742
// available on all supported versions of Node.js.
38-
template <typename FreeType>
43+
template <typename FreeType,
44+
napi_finalize finalizer = default_finalizer<FreeType>>
3945
inline napi_status AttachData(napi_env env,
4046
napi_value obj,
4147
FreeType* data,
42-
napi_finalize finalizer = nullptr,
4348
void* hint = nullptr) {
4449
napi_status status;
45-
if (finalizer == nullptr) {
46-
finalizer = [](napi_env /*env*/, void* data, void* /*hint*/) {
47-
delete static_cast<FreeType*>(data);
48-
};
49-
}
5050
#if (NAPI_VERSION < 5)
5151
napi_value symbol, external;
5252
status = napi_create_symbol(env, nullptr, &symbol);
@@ -1636,11 +1636,8 @@ inline void Object::AddFinalizer(Finalizer finalizeCallback, T* data) const {
16361636
new details::FinalizeData<T, Finalizer>(
16371637
{std::move(finalizeCallback), nullptr});
16381638
napi_status status =
1639-
details::AttachData(_env,
1640-
*this,
1641-
data,
1642-
details::FinalizeData<T, Finalizer>::Wrapper,
1643-
finalizeData);
1639+
details::AttachData<T, details::FinalizeData<T, Finalizer>::Wrapper>(
1640+
_env, *this, data, finalizeData);
16441641
if (status != napi_ok) {
16451642
delete finalizeData;
16461643
NAPI_THROW_IF_FAILED_VOID(_env, status);
@@ -1654,12 +1651,9 @@ inline void Object::AddFinalizer(Finalizer finalizeCallback,
16541651
details::FinalizeData<T, Finalizer, Hint>* finalizeData =
16551652
new details::FinalizeData<T, Finalizer, Hint>(
16561653
{std::move(finalizeCallback), finalizeHint});
1657-
napi_status status = details::AttachData(
1658-
_env,
1659-
*this,
1660-
data,
1661-
details::FinalizeData<T, Finalizer, Hint>::WrapperWithHint,
1662-
finalizeData);
1654+
napi_status status = details::
1655+
AttachData<T, details::FinalizeData<T, Finalizer, Hint>::WrapperWithHint>(
1656+
_env, *this, data, finalizeData);
16631657
if (status != napi_ok) {
16641658
delete finalizeData;
16651659
NAPI_THROW_IF_FAILED_VOID(_env, status);

test/common/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ async function whichBuildType () {
148148
if (await checkBuildType(envBuildType)) {
149149
buildType = envBuildType;
150150
} else {
151-
throw new Error(`The ${envBuildType} build doesn't exists.`);
151+
throw new Error(`The ${envBuildType} build doesn't exist.`);
152152
}
153153
} else {
154154
throw new Error('Invalid value for NODE_API_BUILD_CONFIG environment variable. It should be set to Release or Debug.');

0 commit comments

Comments
 (0)