Skip to content

Node.js binding: support basicarray/structarray parameters and class inheritance (V8 API) - #239

Merged
gangatp merged 3 commits into
Autodesk:developfrom
YackerYan:jingfu/node-binding-support
Sep 30, 2026
Merged

gangatp merged 3 commits into
Autodesk:developfrom
YackerYan:jingfu/node-binding-support

Conversation

@YackerYan

@YackerYan YackerYan commented Sep 29, 2026 •

Copy link
Copy Markdown

Fixes #77

Summary

This PR improves the existing Node.js binding generator (still based on the native V8 / node.h API, not N-API).

basicarray / structarray parameters

  • Implement basicarray and structarray for 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 as 0, nullptr stubs.
  • Add helpers that convert basic array elements to and from V8 values for all integer types, bool, single, double and enum. uint64 / int64 elements are returned as strings to keep full precision; strings or numbers are accepted as input.

Class inheritance (#77)

  • Generated wrapper classes now derive from their IDL parent's wrapper instead of always deriving from CBaseClass.
  • Each class keeps its FunctionTemplate and inherits from its parent's template, so methods of parent classes can be called on derived objects (e.g. MoveNext on an iterator subclass).
  • InitAll initializes parent classes before children. This relies on checkClasses, which already rejects IDLs where a parent class is defined after its child.

Compatibility fixes for newer V8 APIs (verified on Node 12)

  • Qualify Value as v8::Value in generated signatures.
  • Replace deprecated V8 calls: add a NewUtf8String helper (String::NewFromUtf8(...).ToLocalChecked()), use context-aware Set(context, ...), and use BooleanValue(isolate).
  • Use EscapableHandleScope in NewInstance so the returned handle stays valid after the scope closes.
  • Replace std::auto_ptr with std::unique_ptr and add the missing <memory> / <vector> includes.
  • Handle pointer values through uintptr_t, and read uint64 / int64 / pointer struct members from strings as well as numbers, so values written out as strings can be passed back in.
  • Struct conversion now validates the input object and throws std::runtime_error. The generated method's try/catch turns this into a JS TypeError before the C ABI call is made, instead of continuing with a partially filled struct.

Behavior changes

  • basicarray / structarray parameters now require a JS array; other values raise a TypeError.
  • uint64 / int64 elements of returned basic arrays are JS strings, consistent with how scalar 64-bit values are already returned.

Test plan

  • Environment: Node 12.x, Windows 11, MSVC (Visual Studio 2026 Professional)
  • Generated the Node binding for <component / IDL> with this branch's ACT and built the addon with node-gyp.
  • Verified from JS:
    • Addon loads; wrapper and enums are available
    • Method taking a basicarray input
    • Method returning a basicarray / structarray output
    • Method taking a structarray input
    • Parent-class methods callable on a derived class instance (NodeJS bindings do not implement inheritance #77)
    • uint64 / pointer values round-trip correctly

- 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
@YackerYan YackerYan changed the title Fix Node.js binding issues Improve Node.js binding: inheritance, arrays, and V8 API compatibility Sep 29, 2026
@YackerYan YackerYan changed the title Improve Node.js binding: inheritance, arrays, and V8 API compatibility Node.js binding: support basicarray/structarray parameters and class inheritance (V8 API) Sep 29, 2026
Comment thread Source/buildbindingnode.go Outdated
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":

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

case enum is not handled here, it works with enumarray.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, removed. basicarray only allows scalar types, and enums go through enumarray, so this branch was unreachable. I also added the missing pointer case.

Comment thread Source/buildbindingnode.go Outdated
valueExpr = fmt.Sprintf("Boolean::New(isolate, %s)", source)
case "single", "double":
valueExpr = fmt.Sprintf("Number::New(isolate, (double) %s)", source)
case "enum":

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here for enum

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed here as well.

gangatp
gangatp previously approved these changes Sep 30, 2026

@gangatp gangatp left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two minor concerns:

  1. Check ToLocalChecked might crash when GET fails.
  2. 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>
@YackerYan

Copy link
Copy Markdown
Author

Thanks! Both addressed:

Array elements are now read with ToLocal, and a failed read throws a TypeError instead of crashing.
Removed the unused enum cases.

@gangatp
gangatp merged commit a911ba5 into Autodesk:develop Sep 30, 2026
15 checks passed
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants