node-api: update headers for better wasm support

[toyobayashi]

Hi, Node-API team @nodejs/node-api

I'm wondering if emnapi could use node-api-headers package instead of maintaining a modified version of headers.

Just a little modification:

• change __wasm32__ to __wasm__ for building wasm64-unknown-emscripten

• expand NAPI_MODULE_EXPORT to __attribute__((used)) (EMSCRIPTEN_KEEPALIVE) to export napi_register_wasm_v1 and node_api_module_get_api_version_v1 if build with Emscripten

• expand NAPI_EXTERN to __attribute__((__import_module__("env"))) if build with Emscripten. Not sure if this should. Currently there is no way to specify a custom module name in wasm import object for imported function which is implemented in JavaScript, all functions imported from JavaScript are under env module by default. If don't do this change, emnapi must require user to provide a instantiateWasm hook to emscripten and add napi module during this hook call, which isn't what instantiateWasm are supposed to be used for, that force users to write loading logic themselves.

  import init from './node-api-wasm-module-compiled-by-emcc.js'
  
  await init({
    instantiateWasm (imports, successCallback) {
      imports.napi = imports.env
      WebAssembly.instantiate(..., imports).then(({ instance, module }) => {
        successCallback(instance, module)
      })
      return {}
    }
  })

• remove __wasm32__ guards on async work and TSFN, because they have been implemented and are available in wasm, even in wasm32-unknown-unknown and wasm32-wasi. It is worth mentioning that I created a PR src: define NAPI_HAS_THREADS to make TSFN available on Emscripten node-addon-api#1283 that added NAPI_HAS_THREADS to Napi::AsyncWorker and Napi::ThreadSafeFunction, now maybe just add it to Napi::AsyncProgressWorker is enough because it's using std::mutex, which requires __EMSCRIPTEN_PTHREADS__ or __wasi__ && _REENTRANT

[nodejs-github-bot]

Review requested:

• @nodejs/node-api

[mhdawson]

@toyobayashi what is the likelyhood that this would affect any existing addons?

[mhdawson]

@cjihrig I'm wondering if you have any thoughts/opinions on the request related to expand NAPI_EXTERN to __attribute__((__import_module__("env"))) if build with Emscripten

[cjihrig]

@mhdawson I don't think I'm familiar enough with this work to have a good opinion.

[devsnek]

expand NAPI_EXTERN to __attribute__((__import_module__("env"))) if build with Emscripten

This very intentionally already expands to __attribute__((__import_module__("napi"))).

What we could do instead is allow you to define your own NAPI_EXTERN with a simple ifndef check. The default namespace would still be napi but you could modify it on a per-build basis.

[toyobayashi]

what is the likelyhood that this would affect any existing addons?

@mhdawson Reverted changes on NAPI_EXTERN. I believe the remaining changes will have no impact on existing addons.

[toyobayashi]

define your own NAPI_EXTERN with a simple ifndef check

Should we add #ifndef NAPI_EXTERN in BUILDING_NODE_EXTENSION? if BUILDING_NODE_EXTENSION is defined (node-gyp default behavior), custom NAPI_EXTERN seems to not work?

node/src/node_api.h

Lines 4 to 11 in 0d03b77

	#ifdef BUILDING_NODE_EXTENSION
	#ifdef _WIN32
	// Building native addon against node
	#define NAPI_EXTERN __declspec(dllimport)
	#elif defined(__wasm__)
	#define NAPI_EXTERN __attribute__((__import_module__("napi")))
	#endif
	#endif

- #ifdef BUILDING_NODE_EXTENSION
+ #if defined(BUILDING_NODE_EXTENSION) && !defined(NAPI_EXTERN)

Sharp is using node-gyp, emnapi and emscripten to build wasm port, the wasm version is already available on StackBlitz WebContainer. Though #ifndef NAPI_EXTERN already exists in js_native_api.h, I think adding && !defined(NAPI_EXTERN) to #ifdef BUILDING_NODE_EXTENSION is still necessary here since it comes before including js_native_api.h. Otherwise would request users who use both node-gyp and emscripten to undef BUILDING_NODE_EXTENSION in addition to define their own NAPI_EXTERN.

[gabrielschulhof]

It makes sense that, if js_native_api.h is included via node_api.h, then NAPI_EXTERN should still be used if already available.

[mhdawson]

@legendecas is going to take another look at this before we land it.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/54549/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/54551/

[nodejs-github-bot]

Landed in b55adfb