Skip to content

lib,src,test,doc: add node:ffi module#62072

Open
cjihrig wants to merge 14 commits intonodejs:mainfrom
cjihrig:ffi
Open

lib,src,test,doc: add node:ffi module#62072
cjihrig wants to merge 14 commits intonodejs:mainfrom
cjihrig:ffi

Conversation

@cjihrig
Copy link
Copy Markdown
Contributor

@cjihrig cjihrig commented Mar 2, 2026

This is not ready for review yet. Just opening to see if this is something the project is still interested in.

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Mar 2, 2026

Review requested:

  • @nodejs/gyp
  • @nodejs/security-wg

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. labels Mar 2, 2026
@anonrig
Copy link
Copy Markdown
Member

anonrig commented Mar 2, 2026

Yes

Qard
Qard reviewed Mar 2, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@Qard Qard left a comment

Choose a reason for hiding this comment

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

Looks very cool! I'm glad to see someone has picked this back up. 🙂

I did a quick review and it all looks good. My only comment is there seems to be a bunch of static strings and a private symbol which we might want to define in env_properties.h.

@ShogunPanda @cjihrig
ffi: Refactor to simpler API.
91d97f7 
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
@ShogunPanda @cjihrig
fix: Various improvements.
fe8170e 
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
@cjihrig cjihrig added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 18, 2026
@github-actions github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Mar 18, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 18, 2026

Failed to start CI
   ⚠  No approving reviews found
   ✘  Refusing to run CI on potentially unsafe PR
https://github.com/nodejs/node/actions/runs/23272564448

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Mar 18, 2026

CI: https://ci.nodejs.org/job/node-test-pull-request/71877/

anonrig
anonrig previously approved these changes Mar 18, 2026
View reviewed changes
@anonrig anonrig added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Mar 18, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 18, 2026
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Mar 18, 2026

CI: https://ci.nodejs.org/job/node-test-pull-request/71878/

@cjihrig
Copy link
Copy Markdown
Contributor Author

cjihrig commented Mar 18, 2026

@anonrig I cancelled that CI run to save resources. I started https://ci.nodejs.org/job/node-test-pull-request/71877/ manually, and it's currently running.

@jasnell jasnell dismissed anonrig’s stale review March 19, 2026 00:26

given that this is still draft and @cjihrig indicated it's not ready for review, a sign off is not yet appropriate.

mcollina
mcollina previously approved these changes Mar 19, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Mar 19, 2026
@jasnell
Copy link
Copy Markdown
Member

jasnell commented Mar 19, 2026

@cjihrig ... definitely interested in seeing ffi land. I was hoping @bengl's prior efforts would make progress. Once you feel this is ready to go, I'll be happy to review.

@ShogunPanda
fix: Various improvements. (#6)
f46b6e0 
* src,deps,test: various improvements

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* deps,test: fixed failures

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* deps,test: fixed failures

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* deps,test: fixed failures

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* deps,test: fixed failures

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* doc,lib: fixed failures

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* deps: fixed failures

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* test: fixed failures

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

---------

Signed-off-by: Paolo Insogna <paolo@cowtech.it>
@ShogunPanda
test: fixed build on Windows (#7)
643d0b2 
* test: fixed build on Windows

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* test: fixed build on Windows

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* test: fixed build on Windows

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* test: fixed build on Windows

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* test: removed useless file

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

---------

Signed-off-by: Paolo Insogna <paolo@cowtech.it>
@cjihrig cjihrig marked this pull request as ready for review April 1, 2026 14:41
mcollina
mcollina approved these changes Apr 1, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina
Copy link
Copy Markdown
Member

mcollina commented Apr 1, 2026

@cjihrig @ShogunPanda this now conflicts, can you rebase?

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 2, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.17%. Comparing base (a9ac9b1) to head (d562ab9).
⚠️ Report is 14 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #62072      +/-   ##
==========================================
- Coverage   89.70%   89.17%   -0.54%     
==========================================
  Files         695      702       +7     
  Lines      214213   216071    +1858     
  Branches    41021    41412     +391     
==========================================
+ Hits       192167   192680     +513     
- Misses      14113    15447    +1334     
- Partials     7933     7944      +11
Files with missing lines Coverage Δ
lib/ffi.js 95.73% <ø> (ø)
lib/internal/bootstrap/realm.js 96.21% <ø> (+<0.01%) ⬆️
lib/internal/process/permission.js 100.00% <ø> (ø)
lib/internal/process/pre_execution.js 98.63% <ø> (+1.40%) ⬆️
src/env.cc 85.45% <ø> (+0.26%) ⬆️
src/ffi/data.cc 5.30% <ø> (ø)
src/ffi/types.cc 0.00% <ø> (ø)
src/node_binding.cc 82.74% <ø> (ø)
src/node_builtins.cc 76.28% <ø> (+0.23%) ⬆️
src/node_ffi.cc 11.84% <ø> (ø)
... and 8 more

... and 30 files with indirect coverage changes

🚀 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.

jasnell
jasnell reviewed Apr 2, 2026
View reviewed changes
jasnell
jasnell reviewed Apr 2, 2026
View reviewed changes
jasnell
jasnell reviewed Apr 2, 2026
View reviewed changes
jasnell
jasnell reviewed Apr 2, 2026
View reviewed changes
jasnell
jasnell reviewed Apr 2, 2026
View reviewed changes
jasnell
jasnell reviewed Apr 2, 2026
View reviewed changes
jasnell
jasnell reviewed Apr 2, 2026
View reviewed changes
jasnell
jasnell reviewed Apr 2, 2026
View reviewed changes
jasnell
jasnell reviewed Apr 2, 2026
View reviewed changes
jasnell
jasnell reviewed Apr 2, 2026
View reviewed changes
@ShogunPanda
src: formatted files
281152e 
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
@ShogunPanda
Copy link
Copy Markdown
Contributor

ShogunPanda commented Apr 2, 2026

@jasnell Thanks for the review, tomorrow (CEST) I'll address all of them.
Does the architecture and API look fine to you?

@ShogunPanda
fixup
0c646c2 
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
@ShogunPanda
fixup
d562ab9 
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
@ShogunPanda ShogunPanda added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 3, 2026
mcollina
mcollina approved these changes Apr 3, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 3, 2026
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Apr 3, 2026

CI: https://ci.nodejs.org/job/node-test-pull-request/72455/

@jasnell
Copy link
Copy Markdown
Member

jasnell commented Apr 3, 2026

... Does the architecture and API look fine to you?

At first glance, looks fine. I can do another review pass in the next day or two

@ShogunPanda ShogunPanda added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 4, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 4, 2026
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Apr 4, 2026

CI: https://ci.nodejs.org/job/node-test-pull-request/72463/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ShogunPanda ShogunPanda ShogunPanda left review comments

@Qard Qard Qard left review comments

@jasnell jasnell jasnell left review comments

@mcollina mcollina mcollina approved these changes

@anonrig anonrig anonrig left review comments

Assignees

No one assigned

Labels

build Issues and PRs related to build files or the CI. dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@cjihrig @nodejs-github-bot @anonrig @jasnell @mcollina @ShogunPanda @Qard