BabylonJS / BabylonJS/JsRuntimeHost
Make the vendored Node-API sources distinguishable from our own code
- Dominant language
- C++
- Stars
- 22
- Forks
- 23
- Avg merge
- 3d 14h
- Merged PRs (30d)
- 6
Description
[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//` 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`
Contributor guide
Research direction
Start by inventorying Core/Node-API/Source, comparing js_native_api_v8.cc, js_native_api_v8_internals.h, and js_native_api_v8.h with their Node.js counterparts. Review #70, #227, the existing [BABYLON-NATIVE-ADDITION] markers, and the upstream references named in the issue. Done means the project has an agreed structure and provenance scheme that distinguishes upstream, modified-upstream, and in-house files.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- developer-experience
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100