Hi! I'm in the process of sending a PR, and to get started I just cloned this repo (on my Ubuntu 18.04.5 x86_64 machine) and ran cargo test. I had some failures that got corrected by installing some dependencies, such as
sudo apt-get install -y g++-4.8 (I discovered this from the CI config)
sudo apt install libclang1
sudo apt install llvm-9
sudo apt install clang-9
Those could/should be added to installation guides in this project's CONTRIBUTING.md.
However, I got stuck on one error with napi-tests (part of cargo test):
> [email protected] test /home/staltz/neon/test/napi
> mocha --timeout 5000 --recursive lib
/home/staltz/neon/test/napi/node_modules/yargs/yargs.js:1163
else throw err
^
Error: /home/staltz/neon/test/napi/native/index.node: undefined symbol: napi_get_all_property_names
at Object.Module._extensions..node (internal/modules/cjs/loader.js:1207:18)
at Module.load (internal/modules/cjs/loader.js:1001:32)
at Function.Module._load (internal/modules/cjs/loader.js:900:14)
at Module.require (internal/modules/cjs/loader.js:1043:19)
at require (internal/modules/cjs/helpers.js:77:18)
at Object.<anonymous> (/home/staltz/neon/test/napi/lib/arrays.js:1:13)
at Module._compile (internal/modules/cjs/loader.js:1157:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:1177:10)
at Module.load (internal/modules/cjs/loader.js:1001:32)
at Function.Module._load (internal/modules/cjs/loader.js:900:14)
at Module.require (internal/modules/cjs/loader.js:1043:19)
at require (internal/modules/cjs/helpers.js:77:18)
at /home/staltz/neon/test/napi/node_modules/mocha/lib/mocha.js:334:36
at Array.forEach (<anonymous>)
at Mocha.loadFiles (/home/staltz/neon/test/napi/node_modules/mocha/lib/mocha.js:331:14)
at Mocha.run (/home/staltz/neon/test/napi/node_modules/mocha/lib/mocha.js:811:10)
at Object.exports.singleRun (/home/staltz/neon/test/napi/node_modules/mocha/lib/cli/run-helpers.js:108:16)
at exports.runMocha (/home/staltz/neon/test/napi/node_modules/mocha/lib/cli/run-helpers.js:142:13)
at Object.exports.handler (/home/staltz/neon/test/napi/node_modules/mocha/lib/cli/run.js:292:3)
at Object.runCommand (/home/staltz/neon/test/napi/node_modules/yargs/lib/command.js:242:26)
at Object.parseArgs [as _parseArgs] (/home/staltz/neon/test/napi/node_modules/yargs/yargs.js:1087:28)
at Object.parse (/home/staltz/neon/test/napi/node_modules/yargs/yargs.js:566:25)
at Object.exports.main (/home/staltz/neon/test/napi/node_modules/mocha/lib/cli/cli.js:68:6)
at Object.<anonymous> (/home/staltz/neon/test/napi/node_modules/mocha/bin/mocha:164:29)
at Module._compile (internal/modules/cjs/loader.js:1157:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:1177:10)
at Module.load (internal/modules/cjs/loader.js:1001:32)
at Function.Module._load (internal/modules/cjs/loader.js:900:14)
at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:74:12)
at internal/main/run_main_module.js:18:47
npm ERR! Test failed. See above for more details.
My local software versions are:
$ npm version
{
npm: '6.13.4',
ares: '1.15.0',
brotli: '1.0.7',
cldr: '35.1',
http_parser: '2.9.3',
icu: '64.2',
llhttp: '2.0.4',
modules: '72',
napi: '5',
nghttp2: '1.40.0',
node: '12.16.0',
openssl: '1.1.1d',
tz: '2019c',
unicode: '12.1',
uv: '1.34.0',
v8: '7.8.279.23-node.31',
zlib: '1.2.11'
}
$ cargo -V
cargo 1.46.0 (149022b1d 2020-07-17)
$ rustc -V
rustc 1.46.0 (04488afe3 2020-08-24)
@staltz hey, do you have interesting to try https://github.com/napi-rs/napi-rs ?
Oh, I think I found a solution! I just kept trying a bunch of other versions of node, and realized that these work or don't work:
Might be worth writing down in the docs what versions of node.js are supported when it comes to N-API. The README.md currently mentions node 10, 12, 14 "LTS" but doesn't specify that only the most recent 10.x (and most recent 12.x and 14.x) are supported.
README.md mentioned also:
If you're using a different version of Node and believe it should be supported, let us know.
It would be great to fully support 12.16.3 which is what nodejs-mobile currently uses, but it's already quite good if Neon without N-API is supported in 12.16.3 (which I believe is true).
@staltz Yes, that method was only recently added to node (and backported to LTS).
The n-api backend is not complete and not officially stabilized yet. We have not made final decisions for its support matrix yet.
The plan is to have it fully documented by the 1.0 release.
If there's demand for older point releases, that method could maybe be feature flagged somehow. We had a less optimal solution until recently when this method was added to n-api.
Sounds good @kjvalencik. I'm also fine if this issue is closed because in practice it has a resolution. I have set up the repo correctly and tests pass.
It's good to know that nodejs-mobile is on a slightly older version. We should take it into account when deciding on a support matrix! Neon only actually uses the napi_get_all_property_names if you do some_js_object.get_property_names(), which is probably not as frequent an operation for native addons as it is in JS code. Perhaps we could omit it entirely instead of providing the old buggy implementation with that hypothetical feature flag.
The support floor is decided by the N-API version we use, and that translates into the Node.js versions listed here: https://nodejs.org/api/n-api.html#n_api_n_api_version_matrix
napi_get_all_property_names is part of N-API 6.