-
Notifications
You must be signed in to change notification settings - Fork 28.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
napi_create_external_arraybuffer implementation requires upward/circular dependency #49522
Comments
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 |
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. |
Then I suspect it's unlikely to happen. LMK if you change your mind but otherwise please close this. |
I'd prefer to keep it open, since it describes a real architectural defect in the code. |
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. |
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:
#32463
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
The text was updated successfully, but these errors were encountered: