Skip to content

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

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

lib,src,test,doc: add node:ffi module#62072
cjihrig wants to merge 13 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

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

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.

@cjihrig cjihrig force-pushed the ffi branch 2 times, most recently from 9462e4d to 631d73a Compare March 10, 2026 03:46
@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

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

anonrig
anonrig previously approved these changes Mar 18, 2026
@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

@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
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
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

@jasnell jasnell dismissed mcollina’s stale review March 19, 2026 03:40

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

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

@cjihrig cjihrig marked this pull request as ready for review April 1, 2026 14:41
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

Codecov Report

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

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #62072      +/-   ##
==========================================
- Coverage   89.71%   89.17%   -0.55%     
==========================================
  Files         695      702       +7     
  Lines      214546   216183    +1637     
  Branches    41082    41436     +354     
==========================================
+ Hits       192487   192780     +293     
- Misses      14116    15444    +1328     
- Partials     7943     7959      +16     
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.09% <ø> (-0.15%) ⬇️
src/node_ffi.cc 11.84% <ø> (ø)
... and 8 more

... and 29 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.

@ShogunPanda
Copy link
Copy Markdown
Contributor

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

@ShogunPanda ShogunPanda added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 3, 2026
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

@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

cjihrig and others added 13 commits April 6, 2026 08:12
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
* 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>
* 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>
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
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 6, 2026
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 6, 2026
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

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

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

Development

Successfully merging this pull request may close these issues.

7 participants