Node.js binding: support basicarray/structarray parameters and class inheritance (V8 API) - #239
Merged
Conversation
- Qualify `Value` with `v8::Value` in `Load%s` signature for compatibility - Initialize parent class templates before children in `InitAll` - Add helpers for basic array element conversion to/from V8 values
gangatp
reviewed
Sep 30, 2026
| case "double": | ||
| code = fmt.Sprintf("%sif (!%s->IsNumber()) throw std::runtime_error(\"Expected number in basicarray element\");\n", spacing, valueName) | ||
| code += fmt.Sprintf("%s%s = (double) %s->NumberValue(isolate->GetCurrentContext()).ToChecked();\n", spacing, target, valueName) | ||
| case "enum": |
Collaborator
There was a problem hiding this comment.
case enum is not handled here, it works with enumarray.
Author
There was a problem hiding this comment.
Thanks, removed. basicarray only allows scalar types, and enums go through enumarray, so this branch was unreachable. I also added the missing pointer case.
gangatp
reviewed
Sep 30, 2026
| valueExpr = fmt.Sprintf("Boolean::New(isolate, %s)", source) | ||
| case "single", "double": | ||
| valueExpr = fmt.Sprintf("Number::New(isolate, (double) %s)", source) | ||
| case "enum": |
gangatp
previously approved these changes
Sep 30, 2026
gangatp
left a comment
Collaborator
There was a problem hiding this comment.
Two minor concerns:
- Check ToLocalChecked might crash when GET fails.
- enum case not used
- Remove unreachable enum cases from basicarray element helpers (basicarray classes are scalar types; enums use enumarray) - Add pointer element support for basicarray, passed as strings like other pointer values - Read array elements with ToLocal instead of ToLocalChecked so a failing Get throws a TypeError instead of aborting the process Co-authored-by: Cursor <cursoragent@cursor.com>
Author
|
Thanks! Both addressed: Array elements are now read with ToLocal, and a failed read throws a TypeError instead of crashing. |
gangatp
approved these changes
Sep 30, 2026
gangatp
pushed a commit
that referenced
this pull request
Sep 30, 2026
…inheritance (V8 API) (#239) * Fix Node.js binding issues - Qualify `Value` with `v8::Value` in `Load%s` signature for compatibility - Initialize parent class templates before children in `InitAll` - Add helpers for basic array element conversion to/from V8 values * Address review comments on Node.js basicarray handling - Remove unreachable enum cases from basicarray element helpers (basicarray classes are scalar types; enums use enumarray) - Add pointer element support for basicarray, passed as strings like other pointer values - Read array elements with ToLocal instead of ToLocalChecked so a failing Get throws a TypeError instead of aborting the process Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Jingfu Yan <jingfu.yan@autodesk.com> Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #77
Summary
This PR improves the existing Node.js binding generator (still based on the native V8 /
node.hAPI, not N-API).basicarray / structarray parameters
basicarrayandstructarrayfor input parameters (JS array → C buffer) and for output / return values (two-call pattern: query the size, then fill the buffer; returned as a JS array). Previously these were emitted as0, nullptrstubs.bool,single,doubleandenum.uint64/int64elements are returned as strings to keep full precision; strings or numbers are accepted as input.Class inheritance (#77)
CBaseClass.FunctionTemplateand inherits from its parent's template, so methods of parent classes can be called on derived objects (e.g.MoveNexton an iterator subclass).InitAllinitializes parent classes before children. This relies oncheckClasses, which already rejects IDLs where a parent class is defined after its child.Compatibility fixes for newer V8 APIs (verified on Node 12)
Valueasv8::Valuein generated signatures.NewUtf8Stringhelper (String::NewFromUtf8(...).ToLocalChecked()), use context-awareSet(context, ...), and useBooleanValue(isolate).EscapableHandleScopeinNewInstanceso the returned handle stays valid after the scope closes.std::auto_ptrwithstd::unique_ptrand add the missing<memory>/<vector>includes.pointervalues throughuintptr_t, and readuint64/int64/pointerstruct members from strings as well as numbers, so values written out as strings can be passed back in.std::runtime_error. The generated method'stry/catchturns this into a JSTypeErrorbefore the C ABI call is made, instead of continuing with a partially filled struct.Behavior changes
basicarray/structarrayparameters now require a JS array; other values raise aTypeError.uint64/int64elements of returned basic arrays are JS strings, consistent with how scalar 64-bit values are already returned.Test plan
basicarrayinputbasicarray/structarrayoutputstructarrayinputuint64/pointervalues round-trip correctly