镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

node-addon-api addons broken by default on Node.js 20.12+ #52229

Description

@addaleax

Version

v20.12.0

Platform

Linux 6.5.0-26-generic #26~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Tue Mar 12 10:22:43 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

node-api

What steps will reproduce the bug?

cd /tmp && nvm install 20.12.0 && npm i --build-from-source kerberos (or any other node-addon-api addon that uses ObjectWrap or similar APIs)

How often does it reproduce? Is there a required condition?

Consistently.

What is the expected behavior? Why is that the expected behavior?

Addon builds like it did prior to 0ac070c.

What do you see instead?

npm ERR! In file included from ../node_modules/node-addon-api/napi.h:3189,
npm ERR!                  from ../src/kerberos.h:11,
npm ERR!                  from ../src/kerberos.cc:1:
npm ERR! ../node_modules/node-addon-api/napi-inl.h: In instantiation of ‘Napi::ObjectWrap<T>::ObjectWrap(const Napi::CallbackInfo&) [with T = node_kerberos::KerberosClient]’:
npm ERR! ../src/kerberos.cc:64:22:   required from here
npm ERR! ../node_modules/node-addon-api/napi-inl.h:4414:21: error: invalid 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*)’} [-fpermissive]
npm ERR!  4414 |   status = napi_wrap(env, wrapper, instance, FinalizeCallback, nullptr, &ref);
npm ERR!       |            ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
npm ERR!       |                     |
npm ERR!       |                     void (*)(napi_env, void*, void*) {aka void (*)(napi_env__*, void*, void*)}
npm ERR! In file included from /home/addaleax/.cache/node-gyp/20.12.0/include/node/node_api.h:12,
npm ERR!                  from ../node_modules/node-addon-api/napi.h:13,

Additional information

This was introduced in 0ac070c / #50060. From the PR description, this seems like intentional breakage, so it may need a solution in node-addon-api, not here. If it was intentional breakage, calling this out more explicitly somewhere in the release notes would have been helpful imo. @gabrielschulhof

Activity

  1. added
    c++Issues and PRs that require attention from people who are familiar with C++.
    node-apiIssues and PRs related to Node-API.
    on Mar 27, 2024
  2. mhdawson commented on Apr 2, 2024

    @mhdawson
    Member

    @addaleax do you have Experiemental enabled? If I remember correctly any such change should only have affected people using the Experiemental option?

  3. mhdawson commented on Apr 2, 2024

    @mhdawson
    Member

    From the PR

    In keeping with the process of introducing new Node-APIs, this feature is guarded by NAPI_EXPERIMENTAL.

    so I think that my assumption that it should only affect addons using NAPI_EXPERIMENTAL is correct.

    @gabrielschulhof can you clarify as well. I think you were waiting on some PRs to make it into Node.js to address issues on the node-addon-api side of things.

  4. richardlau commented on Apr 2, 2024

    @richardlau
    Member

    If the issue is that #50060 wasn't highlighted in the release notes for 20.12.0, well that had over 400 commits and #50060 wasn't labelled as a notable-change (or even semver-minor). I only highlighted the equivalent in the release notes for Node.js 18.20.0 because it had an explicit backport PR.

  5. addaleax commented on Apr 2, 2024

    @addaleax
    MemberAuthor

    From the PR

    In keeping with the process of introducing new Node-APIs, this feature is guarded by NAPI_EXPERIMENTAL.

    so I think that my assumption that it should only affect addons using NAPI_EXPERIMENTAL is correct.

    @mhdawson Yeah no, the opt-out of this behavior is experimental (behind defined(NAPI_EXPERIMENTAL) && defined(NODE_API_EXPERIMENTAL_NOGC_ENV_OPT_OUT)). The PR description seemed to have been accurate for an initial version of the PR, though.

    do you have Experiemental enabled?

    There’s a self-contained one-liner reproduction in the issue description. (Also ran this in a Docker image to make sure my personal environment is not having any effect.)

    @richardlau Yeah, I was more thinking about the fact that #50060 should have been labeled as a breaking change, not that this should have been caught during the release 🙂

  6. mhdawson commented on Apr 2, 2024

    @mhdawson
    Member

    Reached out to Gabriel through twitter message as well. I think there was likely a mix-up in the guards versus it being planned to affect existing addons not using NAPI_EXPERIMENTAL but would like to confirm with him.

  7. mhdawson commented on Apr 3, 2024

    @mhdawson
    Member

    @richardlau I've not managed to get in touch with @gabrielschulhof yet, but I'm wondering if we should revert the change on 20.x and 18.x. Having done the releases to do forsee any complications with that?

  8. richardlau commented on Apr 3, 2024

    @richardlau
    Member

    @mhdawson Would it be possible for the node-api team to discuss on Friday's meeting if reverting is something that should be done and open revert PRs against 20.x and 18.x if so?

    FWIW for Node.js 18.x we have already have other fixes for regressions that warrant a release:

  9. mhdawson commented on Apr 4, 2024

    @mhdawson
    Member

    @richardlau thanks for confirming that. I'll make sure that we discuss Friday in the team meeintg and if we agree we should revert I'm happy to volunteer to create the revert PRs unless that is something the releaser can easily do as part of the release process.

  10. gabrielschulhof commented on Apr 5, 2024

    @gabrielschulhof
    Contributor

    @addaleax It was not our intention to break add-ons built without NAPI_EXPERIMENTAL.

    At https://github.com/mongodb-js/kerberos/blob/main/src/kerberos.h#L7-L9 NAPI_EXPERIMENTAL is defined. Thus, the add-on is exposed to experimental APIs, including the new nogc types. To get the add-on to build, it is sufficient to either

    1. run the command as CXXFLAGS='-DNODE_API_EXPERIMENTAL_NOGC_ENV_OPT_OUT' npm i --build-from-source, or to
    2. make any of the following changes to the add-on's code:
      1. keep NAPI_EXPERIMENTAL and add the NODE_API_EXPERIMENTAL_NOGC_ENV_OPT_OUT flag, or
      2. bump NAPI_VERSION to 7 and remove NAPI_EXPERIMENTAL, or
      3. do the work to support nogc types.

    This should allow you to move past building the add-on. Thank you for being vigilant, and please keep an eye out for add-ons that break as a result of this change, because it is the first time that we're changing the API signature of otherwise stable APIs, even if we're keeping the ABI the same.

  11. added a commit that references this issue on Apr 19, 2024
  12. added a commit that references this issue on Oct 28, 2024
  13. github-actions commented on May 22, 2026

    @github-actions
    Contributor

    This issue has been marked as stale due to 210 days of inactivity.
    It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.

  14. added
    staleIssues and PRs marked stale due to inactivity and scheduled for automatic closure.
    on May 22, 2026
  15. github-actions commented on Jun 21, 2026

    @github-actions
    Contributor

    This issue has been automatically closed after 30 days of inactivity following its stale status (no activity for a total of 240 days).
    If this is still relevant, feel free to reopen it or leave a comment with additional details so we can continue the discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    c++Issues and PRs that require attention from people who are familiar with C++.node-apiIssues and PRs related to Node-API.staleIssues and PRs marked stale due to inactivity and scheduled for automatic closure.

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions