Skip to content

Commit

Permalink
fix: missing node_api_nogc_env definition (#1585)
Browse files Browse the repository at this point in the history
  • Loading branch information
legendecas authored Oct 8, 2024
1 parent f3eaae6 commit 6ba3891
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 20 deletions.
71 changes: 71 additions & 0 deletions .github/workflows/node-api-headers.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
name: Node.js CI with node-api-headers

on: [push, pull_request]

env:
PYTHON_VERSION: '3.11'

permissions:
contents: read

jobs:
test:
timeout-minutes: 30
strategy:
fail-fast: false
matrix:
api_version:
- '9'
node-version:
- 22.x
node-api-headers-version:
- '1.1.0'
- '1.2.0'
- '1.3.0'
os:
- ubuntu-latest
compiler:
- gcc
- clang
runs-on: ${{ matrix.os }}
steps:
- name: Harden Runner
uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1
with:
egress-policy: audit

- uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
- name: Set up Python ${{ env.PYTHON_VERSION }}
uses: actions/setup-python@f677139bbe7f9c59b41e40162b753c062f5d49a3 # v5.2.0
with:
python-version: ${{ env.PYTHON_VERSION }}
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@0a44ba7841725637a19e28fa30b79a866c81b0a6 # v4.0.4
with:
node-version: ${{ matrix.node-version }}
- name: Check Node.js installation
run: |
node --version
npm --version
- name: Install dependencies
run: |
npm install
npm install "node-api-headers@${{ matrix.node-api-headers-version }}"
- name: npm test
run: |
export NAPI_VERSION=${{ matrix.api_version }}
if [ "${{ matrix.compiler }}" = "gcc" ]; then
export CC="gcc" CXX="g++"
fi
if [ "${{ matrix.compiler }}" = "clang" ]; then
export CC="clang" CXX="clang++"
fi
echo "CC=\"$CC\" CXX=\"$CXX\""
echo "$CC --version"
$CC --version
echo "$CXX --version"
$CXX --version
export CFLAGS="$CFLAGS -O3 --coverage" LDFLAGS="$LDFLAGS --coverage"
export use_node_api_headers=true
echo "CFLAGS=\"$CFLAGS\" LDFLAGS=\"$LDFLAGS\""
npm run pretest -- --verbose
31 changes: 16 additions & 15 deletions napi-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ namespace details {
constexpr int napi_no_external_buffers_allowed = 22;

template <typename FreeType>
inline void default_basic_finalizer(node_api_nogc_env /*env*/,
inline void default_basic_finalizer(node_addon_api_basic_env /*env*/,
void* data,
void* /*hint*/) {
delete static_cast<FreeType*>(data);
Expand All @@ -45,8 +45,9 @@ inline void default_basic_finalizer(node_api_nogc_env /*env*/,
// garbage-collected.
// TODO: Replace this code with `napi_add_finalizer()` whenever it becomes
// available on all supported versions of Node.js.
template <typename FreeType,
node_api_nogc_finalize finalizer = default_basic_finalizer<FreeType>>
template <
typename FreeType,
node_addon_api_basic_finalize finalizer = default_basic_finalizer<FreeType>>
inline napi_status AttachData(napi_env env,
napi_value obj,
FreeType* data,
Expand Down Expand Up @@ -192,10 +193,10 @@ template <typename T, typename Finalizer, typename Hint = void>
struct FinalizeData {
#ifdef NODE_API_EXPERIMENTAL_HAS_POST_FINALIZER
template <typename F = Finalizer,
typename =
std::enable_if_t<std::is_invocable_v<F, node_api_nogc_env, T*>>>
typename = std::enable_if_t<
std::is_invocable_v<F, node_addon_api_basic_env, T*>>>
#endif
static inline void Wrapper(node_api_nogc_env env,
static inline void Wrapper(node_addon_api_basic_env env,
void* data,
void* finalizeHint) NAPI_NOEXCEPT {
WrapVoidCallback([&] {
Expand All @@ -208,9 +209,9 @@ struct FinalizeData {
#ifdef NODE_API_EXPERIMENTAL_HAS_POST_FINALIZER
template <typename F = Finalizer,
typename = std::enable_if_t<
!std::is_invocable_v<F, node_api_nogc_env, T*>>,
!std::is_invocable_v<F, node_addon_api_basic_env, T*>>,
typename = void>
static inline void Wrapper(node_api_nogc_env env,
static inline void Wrapper(node_addon_api_basic_env env,
void* data,
void* finalizeHint) NAPI_NOEXCEPT {
#ifdef NODE_ADDON_API_REQUIRE_BASIC_FINALIZERS
Expand All @@ -228,9 +229,9 @@ struct FinalizeData {
#ifdef NODE_API_EXPERIMENTAL_HAS_POST_FINALIZER
template <typename F = Finalizer,
typename = std::enable_if_t<
std::is_invocable_v<F, node_api_nogc_env, T*, Hint*>>>
std::is_invocable_v<F, node_addon_api_basic_env, T*, Hint*>>>
#endif
static inline void WrapperWithHint(node_api_nogc_env env,
static inline void WrapperWithHint(node_addon_api_basic_env env,
void* data,
void* finalizeHint) NAPI_NOEXCEPT {
WrapVoidCallback([&] {
Expand All @@ -243,9 +244,9 @@ struct FinalizeData {
#ifdef NODE_API_EXPERIMENTAL_HAS_POST_FINALIZER
template <typename F = Finalizer,
typename = std::enable_if_t<
!std::is_invocable_v<F, node_api_nogc_env, T*, Hint*>>,
!std::is_invocable_v<F, node_addon_api_basic_env, T*, Hint*>>,
typename = void>
static inline void WrapperWithHint(node_api_nogc_env env,
static inline void WrapperWithHint(node_addon_api_basic_env env,
void* data,
void* finalizeHint) NAPI_NOEXCEPT {
#ifdef NODE_ADDON_API_REQUIRE_BASIC_FINALIZERS
Expand Down Expand Up @@ -576,9 +577,9 @@ inline Maybe<T> Just(const T& t) {
// BasicEnv / Env class
////////////////////////////////////////////////////////////////////////////////

inline BasicEnv::BasicEnv(node_api_nogc_env env) : _env(env) {}
inline BasicEnv::BasicEnv(node_addon_api_basic_env env) : _env(env) {}

inline BasicEnv::operator node_api_nogc_env() const {
inline BasicEnv::operator node_addon_api_basic_env() const {
return _env;
}

Expand Down Expand Up @@ -5034,7 +5035,7 @@ inline napi_value ObjectWrap<T>::StaticSetterCallbackWrapper(
}

template <typename T>
inline void ObjectWrap<T>::FinalizeCallback(node_api_nogc_env env,
inline void ObjectWrap<T>::FinalizeCallback(node_addon_api_basic_env env,
void* data,
void* /*hint*/) {
// If the child class does not override _any_ Finalize() method, `env` will be
Expand Down
19 changes: 15 additions & 4 deletions napi.h
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,14 @@ template <typename T>
using MaybeOrValue = T;
#endif

#ifdef NODE_API_EXPERIMENTAL_HAS_POST_FINALIZER
using node_addon_api_basic_env = node_api_nogc_env;
using node_addon_api_basic_finalize = node_api_nogc_finalize;
#else
using node_addon_api_basic_env = napi_env;
using node_addon_api_basic_finalize = napi_finalize;
#endif

/// Environment for Node-API values and operations.
///
/// All Node-API values and operations must be associated with an environment.
Expand All @@ -313,16 +321,17 @@ using MaybeOrValue = T;
/// corresponds to an Isolate.
class BasicEnv {
private:
node_api_nogc_env _env;
node_addon_api_basic_env _env;
#if NAPI_VERSION > 5
template <typename T>
static void DefaultFini(Env, T* data);
template <typename DataType, typename HintType>
static void DefaultFiniWithHint(Env, DataType* data, HintType* hint);
#endif // NAPI_VERSION > 5
public:
BasicEnv(node_api_nogc_env env);
operator node_api_nogc_env() const;
BasicEnv(node_addon_api_basic_env env);

operator node_addon_api_basic_env() const;

// Without these operator overloads, the error:
//
Expand Down Expand Up @@ -2469,7 +2478,9 @@ class ObjectWrap : public InstanceWrap<T>, public Reference<Object> {
napi_callback_info info);
static napi_value StaticSetterCallbackWrapper(napi_env env,
napi_callback_info info);
static void FinalizeCallback(node_api_nogc_env env, void* data, void* hint);
static void FinalizeCallback(node_addon_api_basic_env env,
void* data,
void* hint);

static void PostFinalizeCallback(napi_env env, void* data, void* hint);

Expand Down
7 changes: 6 additions & 1 deletion test/binding.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
'value_type_cast.cc'
],
'want_coverage': '<!(node -p process.env.npm_config_coverage)',
'use_node_api_headers': '<!(node -p process.env.use_node_api_headers)',
'conditions': [
['disable_deprecated!="true"', {
'build_sources': ['object/object_deprecated.cc']
Expand All @@ -99,7 +100,11 @@
['want_coverage=="true" and OS=="linux"', {
'cflags_cc': ['--coverage'],
'ldflags': ['--coverage'],
}]
}],
['use_node_api_headers=="true"', {
# prepend to the include_dirs list
'include_dirs+': ["<!(node -p \"require('node-api-headers').include_dir\")"],
}],
],
},
'targets': [
Expand Down

0 comments on commit 6ba3891

Please sign in to comment.