Skip to content

feat: add support for SharedArrayBuffer in DataViews#1714

Open
KevinEady wants to merge 1 commit intonodejs:mainfrom
KevinEady:add-dataview-with-sharedarraybuffer
Open

feat: add support for SharedArrayBuffer in DataViews#1714
KevinEady wants to merge 1 commit intonodejs:mainfrom
KevinEady:add-dataview-with-sharedarraybuffer

Conversation

@KevinEady
Copy link
Contributor

  • Add support for passing Napi::SharedArrayBuffer into Napi::DataView::New() overloads
  • Add DataView::Buffer()
  • Update docs and tests

@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 63.54%. Comparing base (fe0c48e) to head (87673ba).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
napi-inl.h 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1714      +/-   ##
==========================================
+ Coverage   63.50%   63.54%   +0.03%     
==========================================
  Files           3        3              
  Lines        2047     2049       +2     
  Branches      728      728              
==========================================
+ Hits         1300     1302       +2     
  Misses        162      162              
  Partials      585      585              

☔ View full report in Codecov by Sentry.
📢 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.

DataView(napi_env env,
napi_value value); ///< Wraps a Node-API value primitive.

Napi::ArrayBuffer ArrayBuffer() const; ///< Gets the backing array buffer.
Copy link
Member

Choose a reason for hiding this comment

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

We can mark this as deprecated and recommend Buffer().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm actually not so sure about marking this as deprecated anymore. If the add-on author knows for a fact that the DataView will always contain an ArrayBuffer and never a SharedArrayBuffer, it just seems like an extra step for the author to write dataview.Buffer().As<SharedArrayBuffer>() ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We discussed in the 13 March 2026 to update the documentation on ///< Gets the backing array buffer. to say what happens with the As() typechecking like mentioned in the dataview.md document.

napi_value value); ///< Wraps a Node-API value primitive.

Napi::ArrayBuffer ArrayBuffer() const; ///< Gets the backing array buffer.
Napi::Value Buffer() const;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Napi::Value Buffer() const;
Napi::Value Buffer() const; ///< Gets the backing ArrayBuffer or SharedArrayBuffer.

@legendecas legendecas moved this from Need Triage to In Progress in Node-API Team Project Mar 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

3 participants