Ensure that neon compiles with all Node-API versions (CI)

[kjvalencik]

Follow-up from https://github.com/neon-bindings/neon/pull/649

It is possible to correctly specify a Node-API version when adding a Node-API function, but incorrectly specify the Node-API version when using the method.

We should add a step to the Github Actions CI job to ensure cargo check passes for all Node-API versions. This can be a separate workflow since it is independent from OS and will run much quicker than other jobs.

It is unlikely that a contributor will incorrectly identify the Node-API version when creating the binding or that it is missed in review. However, it is likely that the version could be incorrectly identified when using the method, especially since uses could be transient. This CI check should adequately resolve the more likely mistake.

[kjvalencik]

In addition, there are a bunch of other feature flags. We should check that all combinations pass cargo check.

[rubiagatra]

Hello @kjvalencik this is beginner-friendly and I want to contribute.

so do you want to add another pipeline for example napi-check.yaml running cargo check?

thank you for your guidance. In the meantime, I will walk through all getting started and documentation.

[kjvalencik]

@rubiagatra Thanks for the offer to contribute! Yes, another pipeline that runs cargo check repeatedly with different combinations of feature flags to ensure that all [valid] combinations work.

There are a lot of feature flags at the moment (hopefully that will be reduced as a few more APIs stabilize) so it might not be realistic, but it would be great to cover many common patterns!

Some examples:

cargo check --no-default-features --features=napi-1
cargo check --no-default-features --features=napi-3
cargo check --no-default-features --features=napi-4
cargo check --no-default-features --features=napi-5
cargo check --no-default-features --features=napi-6
cargo check --no-default-features --features=napi-experimental
cargo check --no-default-features --features=napi-1,proc-macros
cargo check --no-default-features --features=napi-1,try-catch-api
cargo check --no-default-features --features=napi-4,channel-api
cargo check --no-default-features --features=napi-6,channel-api

It's also worth noting that some feature flags have minimum Node-API versions. For example, enabling channel-api without at least napi-4 will not have any impact.

[deepanshu44]

@kjvalencik I created a CI job prototype here, and you can view its corresponding run here. Please confirm whether I am moving in the right direction.

[kjvalencik]

@deepanshu44 That looks neat! Ideally we would check all permutations of feature flags as well. Using Github Actions matrix building is a cool idea; however, I think it's also worth testing all in one job. It might be faster in a single job because they can re-use the build cache.

[deepanshu44]

@kjvalencik Hmm... Is this is what you're looking for? (logic explained below) This file executes a script called rust.sh.

rust.sh


Here, you simply pass two arguments to the script, one is comma separated list of napi versions (1,2,3...) and the other one is comma separated list of flags (proc-macros,proc-macros...). If a flag has minimum Node-API version, then you assign it in the flag list as follows channel-api:4. Now a nested loop is executed and, as you said, we check all permutations of flags. If a flag has minimum napi version of 4, then napi versions 1,2 and 3 are escaped for that flag.

[kjvalencik]

@deepanshu44 That will create all combinations of Node-API versions and each individual feature flag, but not all permutations of feature flags.

Some feature flags should probably be removed and made default since they have stabilized or are close to stabilization, which would make this a bit lighter weight.

[deepanshu44]

@kjvalencik Please look at the new changes that I made - script, workflow run, yaml file I also need to have a list of all napi versions and their flags. Sorry for the foolish mistake that I made previously.