search

nodejs / node-api-headers

Repository used to make the N-API headers more accessible
MIT License
33 stars 13 forks source link

Experimental `node_api_nogc_env` is not stripped #51

Open legendecas opened 2 weeks ago

legendecas commented 2 weeks ago

Ref: https://github.com/nodejs/node-api-headers/blob/1294543dc1487389acc9cf2371572f57e1eb797d/include/js_native_api_types.h#L46-L53

All other NAPI_EXPERIMENTAL apis were stripped, but node_api_nogc_env and node_api_nogc_finalize were not.

If NAPI_EXPERIMENTAL is defined, node_api_post_finalizer is missing for node_api_nogc_finalize callbacks.

KevinEady commented 1 week ago

Root problem: the current experimental stripping can only support #if(n)def NAPI_EXPERIMENTAL, and so the conditional here is a bit too complex: https://github.com/nodejs/node-api-headers/blob/1294543dc1487389acc9cf2371572f57e1eb797d/scripts/update-headers.js#L49

KevinEady commented 6 days ago

We discussed in the 11 Oct Node-API meeting a possible solution of modifying the Node.js header files such that it would be easier for the tool to correctly remove experimental features.

Thinking about it some more, I am not a big fan of this approach. I don't think we should "dumb down" the header files just because the tool isn't smart enough.

I am looking into making the tool more robust and handling #if as well.

legendecas commented 4 days ago

Just a idea: do we really need to strip experimental APIs?

If the experimental APIs were not stripped, it could be tested in https://github.com/nodejs/node-addon-api/pull/1585 as well.

NickNaso commented 1 day ago

What I remember about the decision to strip experimental APIs is that we would like to avoid developers use APIs that could be removed and then breaks their build.