Fixes issue with deprecated info.Holder() in latest version of V8. Adds support for Node.js 23 and 24 tests.

[agracio]

Changes

• Merged Support Node 23 #979
• Tests accessors-test.js and methodswithdata-test.js are executed using 'old' codebase for Node.js <23.
• weak-test.js and weak2-test.js are ignored for Node.js >23. Reason: Weak tests incompatible with V8 change #995.
• Removed Windows 2019 based tests from GitHub CI. Reason: Windows 2019 is EOL on GitHub Actions and cannot be executed.

Summary

• Fixes error C2039: 'Holder': is not a member of 'v8::FunctionCallbackInfo<v8::Value>' when using electron 36.0.0-beta.4 #996. This error is also present when compiling Node.js 24 modules.
• Adds Node.js 23 and 24 tests to GitHub Actions and AppVeyor.

Tests

• GitHub Actions: https://github.com/agracio/nan/actions/workflows/ci.yml
• AppVeyor: https://ci.appveyor.com/project/agracio/nan/history

[agracio]

I think this is better?

[agracio]

So far CI looks good.
In terms of code quality I know that my handling of conditional test execution depending on Node.js version is not perfect.

[agracio]

Found an error in code that was affecting Node.js 8 and only visible in AppVeyor. Please rerun CI.

[agracio]

Since GitHub Actions can no longer run tests for node.js 8 - 14 maybe it is worth putting them back on AppVeyor.
Newer versions can be removed to make AppVeyor run faster.

This is what happens when attempting to to run Windows 2019 on GitHub Actions: https://github.com/agracio/nan/actions/runs/15712930260

For GitHub Actions adding Windows ARM runner would be beneficial.

  - node-version: lts/*
    os: windows-11-arm  # Windows on arm64

[cclauss]

Given that this repo is in the Nodejs domain, I would vote that all versions < v20 should no longer be tested or supported.

• https://nodejs.org/en/about/previous-releases

[agracio]

Maintaining compatibility with at least 16 and 18 would be great especially considering that 18 LTS support was dropped just a couple of months ago.

Individual developers or small teams usually adapt quickly to Node.js changes but larger organisations might need support for a much longer period.

[cclauss]

If users are paying their Operating System vendor (or someone else) for longer-term support, then that vendor will solve their support issues. Users who are not paying for support should keep up to date to reduce exposure to known vulnerabilities.

[agracio]

Keep in mind that I am not a part of nan or node.js team - but I do not necessarily agree with this position.
Developers need time to adjust and update to newer versions of node.js, things like that do not happen overnight.

Besides if nan can maintain compatibility with older versions without encountering major problems like for example GitHub Actions testing compatibility there is no reason to drop support for older versions.

The biggest problem nan has with older versions of node.js (at least as I see it) is maintaining code compatible with older/newer versions of V8 and node.js leading to countless #if statements that pollute the code and make development considerably more complex than needed.

It was one of those #if statements that led me to make a mistake in code which was not caught by anything except my version of AppVeyor after dropping tests from GitHub CI.

[cclauss]

Developers need time to adjust and update to newer versions of node.js, things like that do not happen overnight.

Then they are free to pin to older versions of their dependencies.

there is no reason to drop support for older versions.

As someone who responds to lots of support requests from developers who refuse to pin and refuse to upgrade, I disagree. This distracts maintainers from making forward progress.

It was one of those #if statements that led me to make a mistake

Exactly my point. Readability, maintainability, and patience all suffer.

[agracio]

I understand your point and do agree that nan should not support endless old versions, but dropping everything except for current LTS sounds extreme. That's why I suggested keeping 16 and 18 or at least 18 as it had its EOL just a couple of months ago.

On a subject of supported versions personally I do not fully understand why older not supported versions are even included in GitHub CI, I refer to 'odd' numbered versions like 17, 19, 21, 23. In my repo I drop them as soon as they are superseded by 'even' numbered LTS candidate.

Unfortunately removing all the #if statements for older node.js and V8 versions would require a significant effort.

To be fair I am not in the position to make any decisions regarding version support so it would be up to you and the rest of the nan maintainers to make that decision.

Nan group does not appear to be very flexible when it comes to dropping old version support, correct me if I'm wrong but versions like 0.12 were only dropped a few months ago.
Personally I do not understand why new nan versions are trying so hard to be backward compatible, in my opinion if anyone needs support for older version of node.js they should use older version of nan.

[kkoopa]

Dropping supported versions would be a breaking change and require a major version bump. That might be what is needed to get this forward, but as much as possible should be supported for as long as possible. No need to cut something if it can be reasonably worked around.

…

On June 18, 2025 2:37:59 AM GMT+03:00, agracio ***@***.***> wrote: agracio left a comment (nodejs/nan#1000) I understand your point and do agree that nan should not support endless old versions, but dropping everything except for current LTS sounds extreme. That's why I suggested keeping 16 and 18, or at least 18 as it had its EOL just a couple of months ago. On a subject of supported versions personally I do not fully understand why older not supported versions are even included in GitHub CI, I refer to 'odd' numbered versions like 17, 19, 21, 23. In my repo I drop them as soon as they are superseded by 'even' numbered LTS candidate. Unfortunately removing all the `#if` statements for older node.js and V8 versions would require a significant effort. -- Reply to this email directly or view it on GitHub: #1000 (comment) You are receiving this because you were mentioned. Message ID: ***@***.***>

[agracio]

@kkoopa What would be the right solution for this PR in order to merge it?

I proposed the following chages:

Since GitHub Actions can no longer run tests for node.js 8 - 14 maybe it is worth putting them back on AppVeyor.
Newer versions can be removed to make AppVeyor run faster. This requires re-enabling AppVeyor.

This is what happens when attempting to to run Windows 2019 on GitHub Actions: https://github.com/agracio/nan/actions/runs/15712930260

For GitHub Actions adding Windows ARM runner would be beneficial.

  - node-version: lts/*
    os: windows-11-arm  # Windows on arm64

[agracio]

@cclauss , @kkoopa I am not very familiar with PR process in nan, is there anything else I need to do before PR can be merged or it just takes time and I am being impatient?

[kkoopa]

There is not much more to it, just takes time. I have not merged #979 because it does not pass the required tests. Just having it compile is not good enough, the observable behavior should also be indistinguishable across all supported versions. If going down the route of breaking backwards compatibility, all old code should also be expunged, affected tests should be updated, the API should be cleaned of deprecated items, it should also be updated to cover whatever is lurking in the bleeding version of v8, to delay the need for a new break as long as possible.

[agracio]

@kkoopa This PR merges #979 and does pass all the required tests including node.js 23 and 24, however it is no longer possible to run Windows 2019 tests on GitHub and they should be offloaded back to AppVeyor.
All Windows 2019 tests for node.js 8, 10, 12 and 14 do pass on AppVeyor.

This PR does not break any backward compatibility.

My suggestion was to remove all newer tests from AppVeyor and leave only 8-14 on it while 16-24 runs on GitHub.

[kkoopa]

Sorry, I missed that you had linked to your own CIs with tests. This does pass all tests, but it changes the contract. I guess it is fine at this point. Could you update the documentation regarding deprecations and changed behavior (under doc, not the README itself, which is autogenerated). Holder should also be deprecated. Did I miss something else affected?

[agracio]

I only merged #979 and fixed tests, not 100% sure what other areas it affects. Could you point me to the doc that needs changing?

Should I remove versions 16-24 from AppVeyor - note that it does require for you to re-enable AppVeyor for nan builds.

[agracio]

Code changes do not explicitly deprecate Nan::Holder() but rather change its behaviour depending on V8 version, not sure if docs need to be updated.

inline v8::Local<v8::Object> Holder() const {
#if defined(V8_MAJOR_VERSION) &&                                               \
    (V8_MAJOR_VERSION > 12 ||                                                  \
     (V8_MAJOR_VERSION == 12 &&                                                \
      (defined(V8_MINOR_VERSION) &&                                            \
       (V8_MINOR_VERSION > 5 ||                                                \
        (V8_MINOR_VERSION == 5 && defined(V8_BUILD_NUMBER) &&                  \
         V8_BUILD_NUMBER >= 214)))))
        return info_.This();
#else
        return info_.Holder();
#endif
}

However Nan::Callee() is marked as deprecated.

NAN_DEPRECATED inline v8::Local<v8::Function> Callee() const {
   return info_.Callee();
}

[kkoopa]

You should rebase this on whatever #979 is. Some changes are mentioned in the commit messages of #979, e.g. 13e72d4, the one about Callee was supposed to have been deprecated a long time ago, but it had been missed, although now I am thinking that it might start spamming deprecation messages on affected versions and that it had been left on purpose, so that commit should go, but I can deal with that when merging. Affected code examples and such might need updating. Some docs reference Holder, e.g. https://github.com/nodejs/nan/blob/main/doc/methods.md#api_nan_function_callback_info, but I guess Holder could be retained for old versions and just mentioned in the documentation that it is the same as This on newer versions.

[agracio]

This PR is is based on #979 with changes only to js tests and to GitHub and Appveyor CI pipelines.

• Merged Support Node 23 #979
• Tests accessors-test.js and methodswithdata-test.js are executed using 'old' codebase for Node.js <23.
• weak-test.js and weak2-test.js are ignored for Node.js >23. Reason: Weak tests incompatible with V8 change #995.
• Removed Windows 2019 based tests from GitHub CI. Reason: Windows 2019 is EOL on GitHub Actions and cannot be executed.

I can undo Callee() deprecation.

https://github.com/nodejs/nan/blob/main/doc/methods.md mentions Callee() in code but also states that it is not available in node.js >= 10

[kkoopa]

I meant, could you retarget this PR to the node23 branch, then I could merge that.

[agracio]

Done

[nikeee]

Can I help merging this PR in any way?

[agracio]

I was asked to merge my changes into node23 branch so there is a new PR but it has seen no activity for weeks #1001.
Not sure what is happening with nan maintainers team but haven't seen any responses to PRs or Issues lately.

You could use my github repo as reference in package.json to solve the problem until changes are merged.

[nikeee]

You could use my github repo as reference in package.json to solve the problem until changes are merged.

That's exactly what I did:

  "overrides": {
    "nan": "github:agracio/nan#a91d752"
  },

Works just fine. Thanks!