Node 18 compatibility #937

mmomtchev · 2022-04-21T11:15:36Z

Compatibility with Node 18.0.0

mkrufky

Thanks for this - looks good!

Please also add Node 18 to Appveyor and TravisCI test matrices (although Travis is not currently being used) as was done in the following commit:

96ed51a

mkrufky · 2022-04-21T13:43:37Z

closes #936

mmomtchev · 2022-04-21T13:47:54Z

@mkrufky done

mkrufky · 2022-04-21T18:28:11Z

And, unfortunately the tests are failing to build:

npm run rebuild-tests && npm test

> nan@2.15.0 rebuild-tests git/nan
> node-gyp rebuild --msvs_version=2015 --directory test

make: Entering directory 'git/nan/test/build'
  CXX(target) Release/obj.target/accessors/cpp/accessors.o
  SOLINK_MODULE(target) Release/obj.target/accessors.node
  COPY Release/accessors.node
  CXX(target) Release/obj.target/accessors2/cpp/accessors2.o
  SOLINK_MODULE(target) Release/obj.target/accessors2.node
  COPY Release/accessors2.node
  CXX(target) Release/obj.target/asyncprogressqueueworker/cpp/asyncprogressqueueworker.o
  SOLINK_MODULE(target) Release/obj.target/asyncprogressqueueworker.node
  COPY Release/asyncprogressqueueworker.node
  CXX(target) Release/obj.target/asyncprogressqueueworkerstream/cpp/asyncprogressqueueworkerstream.o
  SOLINK_MODULE(target) Release/obj.target/asyncprogressqueueworkerstream.node
  COPY Release/asyncprogressqueueworkerstream.node
  CXX(target) Release/obj.target/asyncprogressworker/cpp/asyncprogressworker.o
  SOLINK_MODULE(target) Release/obj.target/asyncprogressworker.node
  COPY Release/asyncprogressworker.node
  CXX(target) Release/obj.target/asyncprogressworkersignal/cpp/asyncprogressworkersignal.o
  SOLINK_MODULE(target) Release/obj.target/asyncprogressworkersignal.node
  COPY Release/asyncprogressworkersignal.node
  CXX(target) Release/obj.target/asyncprogressworkerstream/cpp/asyncprogressworkerstream.o
  SOLINK_MODULE(target) Release/obj.target/asyncprogressworkerstream.node
  COPY Release/asyncprogressworkerstream.node
  CXX(target) Release/obj.target/asyncresource/cpp/asyncresource.o
  SOLINK_MODULE(target) Release/obj.target/asyncresource.node
  COPY Release/asyncresource.node
  CXX(target) Release/obj.target/asyncworker/cpp/asyncworker.o
  SOLINK_MODULE(target) Release/obj.target/asyncworker.node
  COPY Release/asyncworker.node
  CXX(target) Release/obj.target/asyncworkererror/cpp/asyncworkererror.o
  SOLINK_MODULE(target) Release/obj.target/asyncworkererror.node
  COPY Release/asyncworkererror.node
  CXX(target) Release/obj.target/buffer/cpp/buffer.o
  SOLINK_MODULE(target) Release/obj.target/buffer.node
  COPY Release/buffer.node
  CXX(target) Release/obj.target/bufferworkerpersistent/cpp/bufferworkerpersistent.o
  SOLINK_MODULE(target) Release/obj.target/bufferworkerpersistent.node
  COPY Release/bufferworkerpersistent.node
  CXX(target) Release/obj.target/callbackcontext/cpp/callbackcontext.o
  SOLINK_MODULE(target) Release/obj.target/callbackcontext.node
  COPY Release/callbackcontext.node
  CXX(target) Release/obj.target/converters/cpp/converters.o
  SOLINK_MODULE(target) Release/obj.target/converters.node
  COPY Release/converters.node
  CXX(target) Release/obj.target/error/cpp/error.o
  SOLINK_MODULE(target) Release/obj.target/error.node
  COPY Release/error.node
  CXX(target) Release/obj.target/gc/cpp/gc.o
  SOLINK_MODULE(target) Release/obj.target/gc.node
  COPY Release/gc.node
  CXX(target) Release/obj.target/indexedinterceptors/cpp/indexedinterceptors.o
  SOLINK_MODULE(target) Release/obj.target/indexedinterceptors.node
  COPY Release/indexedinterceptors.node
  CXX(target) Release/obj.target/isolatedata/cpp/isolatedata.o
  SOLINK_MODULE(target) Release/obj.target/isolatedata.node
  COPY Release/isolatedata.node
  CXX(target) Release/obj.target/makecallback/cpp/makecallback.o
  SOLINK_MODULE(target) Release/obj.target/makecallback.node
  COPY Release/makecallback.node
  CXX(target) Release/obj.target/maybe/cpp/maybe.o
  SOLINK_MODULE(target) Release/obj.target/maybe.node
  COPY Release/maybe.node
  CXX(target) Release/obj.target/methodswithdata/cpp/methodswithdata.o
  SOLINK_MODULE(target) Release/obj.target/methodswithdata.node
  COPY Release/methodswithdata.node
  CXX(target) Release/obj.target/morenews/cpp/morenews.o
  SOLINK_MODULE(target) Release/obj.target/morenews.node
  COPY Release/morenews.node
  CXX(target) Release/obj.target/multifile/cpp/multifile1.o
  CXX(target) Release/obj.target/multifile/cpp/multifile2.o
  SOLINK_MODULE(target) Release/obj.target/multifile.node
  COPY Release/multifile.node
  CXX(target) Release/obj.target/namedinterceptors/cpp/namedinterceptors.o
  SOLINK_MODULE(target) Release/obj.target/namedinterceptors.node
  COPY Release/namedinterceptors.node
  CXX(target) Release/obj.target/nancallback/cpp/nancallback.o
  SOLINK_MODULE(target) Release/obj.target/nancallback.node
  COPY Release/nancallback.node
  CXX(target) Release/obj.target/nannew/cpp/nannew.o
  SOLINK_MODULE(target) Release/obj.target/nannew.node
  COPY Release/nannew.node
  CXX(target) Release/obj.target/news/cpp/news.o
../cpp/news.cpp: In function ‘Nan::NAN_METHOD_RETURN_TYPE NewScript2(Nan::NAN_METHOD_ARGS_TYPE)’:
../cpp/news.cpp:121:42: error: no matching function for call to ‘v8::ScriptOrigin::ScriptOrigin(v8::Isolate*, v8::Local<v8::String>)’
     New<v8::String>("x").ToLocalChecked());
                                          ^
In file included from .node-gyp/9.11.2/include/node/node.h:63:0,
                 from ../../nan.h:60,
                 from ../cpp/news.cpp:9:
.node-gyp/9.11.2/include/node/v8.h:9592:1: note: candidate: v8::ScriptOrigin::ScriptOrigin(v8::Local<v8::Value>, v8::Local<v8::Integer>, v8::Local<v8::Integer>, v8::Local<v8::Boolean>, v8::Local<v8::Integer>, v8::Local<v8::Value>, v8::Local<v8::Boolean>, v8::Local<v8::Boolean>, v8::Local<v8::Boolean>)
 ScriptOrigin::ScriptOrigin(Local<Value> resource_name,
 ^~~~~~~~~~~~
.node-gyp/9.11.2/include/node/v8.h:9592:1: note:   no known conversion for argument 1 from ‘v8::Isolate*’ to ‘v8::Local<v8::Value>’
.node-gyp/9.11.2/include/node/v8.h:1062:7: note: candidate: constexpr v8::ScriptOrigin::ScriptOrigin(const v8::ScriptOrigin&)
 class ScriptOrigin {
       ^~~~~~~~~~~~
.node-gyp/9.11.2/include/node/v8.h:1062:7: note:   candidate expects 1 argument, 2 provided
.node-gyp/9.11.2/include/node/v8.h:1062:7: note: candidate: constexpr v8::ScriptOrigin::ScriptOrigin(v8::ScriptOrigin&&)
.node-gyp/9.11.2/include/node/v8.h:1062:7: note:   candidate expects 1 argument, 2 provided
../cpp/news.cpp: In function ‘Nan::NAN_METHOD_RETURN_TYPE CompileScript2(Nan::NAN_METHOD_ARGS_TYPE)’:
../cpp/news.cpp:142:42: error: no matching function for call to ‘v8::ScriptOrigin::ScriptOrigin(v8::Isolate*, v8::Local<v8::String>)’
     New<v8::String>("x").ToLocalChecked());
                                          ^
In file included from .node-gyp/9.11.2/include/node/node.h:63:0,
                 from ../../nan.h:60,
                 from ../cpp/news.cpp:9:
.node-gyp/9.11.2/include/node/v8.h:9592:1: note: candidate: v8::ScriptOrigin::ScriptOrigin(v8::Local<v8::Value>, v8::Local<v8::Integer>, v8::Local<v8::Integer>, v8::Local<v8::Boolean>, v8::Local<v8::Integer>, v8::Local<v8::Value>, v8::Local<v8::Boolean>, v8::Local<v8::Boolean>, v8::Local<v8::Boolean>)
 ScriptOrigin::ScriptOrigin(Local<Value> resource_name,
 ^~~~~~~~~~~~
.node-gyp/9.11.2/include/node/v8.h:9592:1: note:   no known conversion for argument 1 from ‘v8::Isolate*’ to ‘v8::Local<v8::Value>’
.node-gyp/9.11.2/include/node/v8.h:1062:7: note: candidate: constexpr v8::ScriptOrigin::ScriptOrigin(const v8::ScriptOrigin&)
 class ScriptOrigin {
       ^~~~~~~~~~~~
.node-gyp/9.11.2/include/node/v8.h:1062:7: note:   candidate expects 1 argument, 2 provided
.node-gyp/9.11.2/include/node/v8.h:1062:7: note: candidate: constexpr v8::ScriptOrigin::ScriptOrigin(v8::ScriptOrigin&&)
.node-gyp/9.11.2/include/node/v8.h:1062:7: note:   candidate expects 1 argument, 2 provided
news.target.mk:101: recipe for target 'Release/obj.target/news/cpp/news.o' failed
make: *** [Release/obj.target/news/cpp/news.o] Error 1
make: Leaving directory 'git/nan/test/build'
gyp ERR! build error 
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (.nvm/versions/node/v9.11.2/lib/node_modules/npm/node_modules/node-gyp/lib/build.js:258:23)
gyp ERR! stack     at ChildProcess.emit (events.js:180:13)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:209:12)
gyp ERR! System Linux 4.16.2-vujade
gyp ERR! command ".nvm/versions/node/v9.11.2/bin/node" ".nvm/versions/node/v9.11.2/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild" "--msvs_version=2015" "--directory" "test"
gyp ERR! cwd git/nan/test
gyp ERR! node -v v9.11.2
gyp ERR! node-gyp -v v3.6.2
gyp ERR! not ok 
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! nan@2.15.0 rebuild-tests: `node-gyp rebuild --msvs_version=2015 --directory test`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the nan@2.15.0 rebuild-tests script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     .npm/_logs/2022-04-21T18_23_03_172Z-debug.log

They just need to be updated to match the new function signatures.

mmomtchev · 2022-04-21T18:32:11Z

@mkrufky There was a typo in the macro used to check the Node version

mkrufky · 2022-04-21T21:59:00Z

Still seeing this error now, running Node 18, only. I suspect it's just a missing version on Appveyor's end, since 18 was only just released.

Build started
git clone -q --branch=node.18 https://github.com/mkrufky/nan.git C:\projects\nan
git checkout -qf 4be3e5c4e93ddf[2](https://ci.appveyor.com/project/mkrufky/nan/build/job/4u6vpbyhhe95q3ii#L2)014b71ad9717078d76c9ed549
Running Install scripts
Update-NodeJsInstallation (Get-NodeJsLatestBuild $env:nodejs_version)
Updating Node.js v18.0.0 (x86)
Uninstalling Node.js v8.17.0 (x86)...
Installing Node.js v18.0.0 (x86)...
Exception calling "DownloadFile" with "2" argument(s): "The remote server returned an error: (404) Not Found."
At C:\Program Files\AppVeyor\BuildAgent\Modules\nodejs-utils\nodejs-utils.psm1:58 char:5
+     (New-Object Net.WebClient).DownloadFile($packageUrl, $packageFile ...
+     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodInvocationException
    + FullyQualifiedErrorId : WebException
 
IF %nodejs_version% LSS 4 npm -g install npm@2
IF %nodejs_version% EQU 5 npm -g install npm@[3](https://ci.appveyor.com/project/mkrufky/nan/build/job/4u6vpbyhhe95q3ii#L3)
set PATH=%APPDATA%\npm;%PATH%
npm install
'npm' is not recognized as an internal or external command,
operable program or batch file.
Command exited with code 1

...and for some reason, I can't get Travis CI to run this repository, even within my own account.

I'll re-run the appveyor tests tomorrow - maybe Appveyor will fix it on their own.

I'll squash+merge this once it passes.

mkrufky · 2022-04-21T22:04:06Z

This is the ticket that I filed on Appveyor:
https://help.appveyor.com/discussions/problems/31957-please-install-nodejs-v18-new-builds-are-failing-looks-like-node-18-is-missing

mkrufky · 2022-04-22T20:20:17Z

depends on #932
depends on #939

Flarna · 2022-05-24T07:55:35Z

Any chance to get this into a release soon?

kkoopa · 2022-05-24T14:13:13Z

Absolutely. Thank you for the reminder. I confess that I completely forgot about it. I will push a new version tonight or tomorrow.

…

On May 24, 2022 10:55:49 AM GMT+03:00, "Gerhard Stöbich" ***@***.***> wrote: Any chance to get this into a release soon?

kkoopa · 2022-05-25T13:07:05Z

It is done.

NAN did have a small PR/release for Node 18 support. Might as well make that our minimum: nodejs/nan#937.

Node 18 compatbiility

09aac66

mkrufky requested changes Apr 21, 2022

View reviewed changes

add Node 18 to CI

c3e72f9

fix a typo

4be3e5c

mmomtchev force-pushed the node18 branch from bb0dc4c to 4be3e5c Compare April 21, 2022 18:33

Qard mentioned this pull request Apr 22, 2022

Add Node.js 18 to CI DataDog/pprof-nodejs#25

Merged

mkrufky merged commit 16fa322 into nodejs:main Apr 23, 2022

mmomtchev deleted the node18 branch April 23, 2022 08:14

michaelgoin added a commit to michaelgoin/node-native-metrics that referenced this pull request Jul 27, 2022

Bumps minimum nan version to ^2.16.0.

65fc29d

NAN did have a small PR/release for Node 18 support. Might as well make that our minimum: nodejs/nan#937.

michaelgoin mentioned this pull request Jul 27, 2022

Bumps minimum nan version to ^2.16.0. newrelic/node-native-metrics#201

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Node 18 compatibility #937

Node 18 compatibility #937

mmomtchev commented Apr 21, 2022

mkrufky left a comment

mkrufky commented Apr 21, 2022

mmomtchev commented Apr 21, 2022

mkrufky commented Apr 21, 2022 •

edited

mmomtchev commented Apr 21, 2022

mkrufky commented Apr 21, 2022

mkrufky commented Apr 21, 2022

mkrufky commented Apr 22, 2022

Flarna commented May 24, 2022

kkoopa commented May 24, 2022 via email

kkoopa commented May 25, 2022

Node 18 compatibility #937

Node 18 compatibility #937

Conversation

mmomtchev commented Apr 21, 2022

mkrufky left a comment

Choose a reason for hiding this comment

mkrufky commented Apr 21, 2022

mmomtchev commented Apr 21, 2022

mkrufky commented Apr 21, 2022 • edited

mmomtchev commented Apr 21, 2022

mkrufky commented Apr 21, 2022

mkrufky commented Apr 21, 2022

mkrufky commented Apr 22, 2022

Flarna commented May 24, 2022

kkoopa commented May 24, 2022 via email

kkoopa commented May 25, 2022

mkrufky commented Apr 21, 2022 •

edited