Repository navigation
napi_create_external_arraybuffer implementation requires upward/circular dependency #49522
Description
Version
18.x-staging head
Platform
No response
Subsystem
Node-API
What steps will reproduce the bug?
Using only the JS portion of the Node API, call napi_create_external_arraybuffer() and note that the ERR_BUFFER_CONTEXT_NOT_AVAILABLE error results.
How often does it reproduce? Is there a required condition?
No response
What is the expected behavior? Why is that the expected behavior?
The expected behaviour is that functions in the JS-only portion of the Node API can be called without requiring the entire Node environment; this breaks separation of concerns.
What do you see instead?
The following change introduced an upward JS Node API dependency on the Node API:
For consumers of the JS-only API who don't use the Node portion of the Node API, calling napi_create_external_arraybuffer results in ERR_BUFFER_CONTEXT_NOT_AVAILABLE.
Reverting the change in the above linked issue caused napi_create_external_arraybuffer to again work correctly.
Because the API implementation has deviated significantly since then, it has become impossible to keep my fork up-to-date, even to the tip of 18.x-staging.
Requesting that this be implemented in JS Node API without requiring an upward dependency on Node API, as it was prior to the above change. This decision makes the JS Node API unusable without all of Node, affects my ability to use Node-API (and requires me to use an old fork), and is a generally broken architectural design.
Failing that, [a] workaround[s] that I can apply to keep my fork up-to-date.
Additional information
No response
Activity
bnoordhuis commentedon Sep 7, 2023
It took me a long time to parse this but I think you're saying you use napi without creating a node runtime, correct?
I don't know if that's a reasonable expectation. I'm not even sure if that's a promise napi makes. I mean, the N stands for Node, not JS.
External arraybuffers need to be tracked (and cleaned up) by something and that's what the CallbackInfo class in src/node_buffer.cc does. napi could maybe1 do that by itself but that results in a lot of duplicated functionality.
I'd say that if you want to see that happen, open a pull request and see how it's received. Maybe you'll find it's simply not technically feasible to break the dependency.
1 or maybe not because of event loop integration
sparist commentedon Sep 7, 2023
That is correct. Some background: I have a V8 embedding that uses Google Dawn for its WebGPU implementation. Its API is coded for Node-API (and only the JS portion). Therefore, I must use Node-API, but I don't want the higher Node layer that includes the event loop etc.
There is another project, BabylonNative, that maintains a fork of the Node JS API for a similar reason.
Maybe not, but it seems to be bad architectural design to allow the JS portion reach up into Node for just this one thing. Upward dependencies are bad.
It seems to me that if the JS layer requires this, the implementation should be moved to the JS layer.
I sadly don't have sufficient time or domain knowledge to code this.
bnoordhuis commentedon Sep 7, 2023
Then I suspect it's unlikely to happen. LMK if you change your mind but otherwise please close this.
sparist commentedon Sep 7, 2023
I'd prefer to keep it open, since it describes a real architectural defect in the code.
bnoordhuis commentedon Sep 7, 2023
That is, with all due deference, just your opinion. For another perspective: it's doing exactly what it's designed to do.
I don't like keeping issues around that aren't going to make progress and I'm not even sure I agree something needs to change so I'll take the liberty of closing it.