[Filed by Copilot on behalf of @bghgary]
Core/Node-API/Source mixes files vendored from Node.js with implementations we wrote, and nothing marks which is which. #227 is what that costs: js_native_api_v8_internals.h looks vendored but is a hand-written shim, and it carried a handle leak from the original 2019 fork until last week.
Syncing does not fix it. #70 ("Update Node-API to latest from node.js") rewrote 54 lines of that same file in 2024 and left the bug in, because the file has no upstream counterpart to copy from — upstream's version pulls in node_internals.h / env.h / util-inl.h, so ours reimplements OneByteString, the CHECK macros and PersistentToLocal locally.
A 3rdparty/<name>/ directory carrying a README or license that links the source would cover the straightforward case, but it doesn't fit cleanly here: we vendor a subset of Node's files and add engine implementations (Chakra, JavaScriptCore, QuickJS) alongside them.
What structure actually works is the open question:
- which files are verbatim upstream, which are modified upstream, and which are ours
- where the upstream ref is recorded, so a future sync knows what it is syncing from
- whether per-change markers are enough — the
[BABYLON-NATIVE-ADDITION] convention is currently 7 lines in js_native_api_v8.cc, 3 in js_native_api_v8_internals.h, and 0 in js_native_api_v8.h
[Filed by Copilot on behalf of @bghgary]
Core/Node-API/Sourcemixes files vendored from Node.js with implementations we wrote, and nothing marks which is which. #227 is what that costs:js_native_api_v8_internals.hlooks vendored but is a hand-written shim, and it carried a handle leak from the original 2019 fork until last week.Syncing does not fix it. #70 ("Update Node-API to latest from node.js") rewrote 54 lines of that same file in 2024 and left the bug in, because the file has no upstream counterpart to copy from — upstream's version pulls in
node_internals.h/env.h/util-inl.h, so ours reimplementsOneByteString, theCHECKmacros andPersistentToLocallocally.A
3rdparty/<name>/directory carrying a README or license that links the source would cover the straightforward case, but it doesn't fit cleanly here: we vendor a subset of Node's files and add engine implementations (Chakra, JavaScriptCore, QuickJS) alongside them.What structure actually works is the open question:
[BABYLON-NATIVE-ADDITION]convention is currently 7 lines injs_native_api_v8.cc, 3 injs_native_api_v8_internals.h, and 0 injs_native_api_v8.h