Skip to content

fix: allow const vectors with Object::New overload utilizing node_api_create_object_with_properties - #1755

Merged
legendecas merged 1 commit into
mainfrom
use-const-cast-with-node_api_create_object_with_properties
Sep 2, 2026
Merged

fix: allow const vectors with Object::New overload utilizing node_api_create_object_with_properties#1755
legendecas merged 1 commit into
mainfrom
use-const-cast-with-node_api_create_object_with_properties

Conversation

@KevinEady

Copy link
Copy Markdown
Contributor

Use const_cast to remove const-qualifier from the pointer returned by the vectors' data() method.

@KevinEady

Copy link
Copy Markdown
Contributor Author

Tested locally against nodejs/node#65621

@codecov-commenter

codecov-commenter commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.68%. Comparing base (06d56f7) to head (12be15f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1755   +/-   ##
=======================================
  Coverage   63.68%   63.68%           
=======================================
  Files           3        3           
  Lines        2065     2065           
  Branches      731      731           
=======================================
  Hits         1315     1315           
  Misses        162      162           
  Partials      588      588           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@KevinEady
KevinEady marked this pull request as ready for review August 28, 2026 18:17
@KevinEady
KevinEady requested a review from legendecas August 28, 2026 18:17
Comment thread napi-inl.h
propertyValues.data(),
propertyNames.size(),
&value);
napi_status status = node_api_create_object_with_properties(

@legendecas legendecas Aug 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we add a TODO to remove the const_cast when nodejs/node#65621 is fully backported?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in push from cefce1a to 12be15f (diff)

@KevinEady
KevinEady force-pushed the use-const-cast-with-node_api_create_object_with_properties branch from cefce1a to 12be15f Compare August 31, 2026 18:29

@legendecas legendecas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! Let's wait a bit until nodejs/node#65621 lands.

@legendecas
legendecas merged commit 25c6b21 into main Sep 2, 2026
87 checks passed
@github-project-automation github-project-automation Bot moved this from Need Triage to Done in Node-API Team Project Sep 2, 2026
@legendecas
legendecas deleted the use-const-cast-with-node_api_create_object_with_properties branch September 2, 2026 14:24
@github-actions github-actions Bot mentioned this pull request Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

3 participants