Skip to content

ffi: remove function signature property aliases#63482

Open
Renegade334 wants to merge 1 commit into
nodejs:mainfrom
Renegade334:ffi-dealias-signatures
Open

ffi: remove function signature property aliases#63482
Renegade334 wants to merge 1 commit into
nodejs:mainfrom
Renegade334:ffi-dealias-signatures

Conversation

@Renegade334
Copy link
Copy Markdown
Member

Refs: https://github.com/nodejs/node/pull/62072/changes#r3067834658

Having multiple property name aliases for function signature objects doesn't add utility, but from a TS perspective, it blocks being able to infer the resultant function type from the shape of the signature object. With all else being equal, it'd be useful to get rid of them.

In the absence of any of these names being unambiguously more appropriate than the others, parameters and result appear to be the most canonical.

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/ffi

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels May 21, 2026
@Renegade334 Renegade334 added the ffi Issues and PRs related to experimental Foreign Function Interface support. label May 21, 2026
Signed-off-by: Renegade334 <contact.9a5d6388@renegade334.me.uk>
@Renegade334 Renegade334 force-pushed the ffi-dealias-signatures branch from 35c2eee to 52cedc7 Compare May 21, 2026 23:15
@codecov
Copy link
Copy Markdown

codecov Bot commented May 22, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.14%. Comparing base (2f56cd2) to head (52cedc7).

Files with missing lines Patch % Lines
lib/internal/ffi-shared-buffer.js 25.00% 3 Missing ⚠️
src/ffi/types.cc 66.66% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #63482      +/-   ##
==========================================
- Coverage   90.17%   90.14%   -0.04%     
==========================================
  Files         718      718              
  Lines      227731   227697      -34     
  Branches    42768    42750      -18     
==========================================
- Hits       205365   205255     -110     
- Misses      14145    14218      +73     
- Partials     8221     8224       +3     
Files with missing lines Coverage Δ
src/ffi/types.cc 46.91% <66.66%> (-2.01%) ⬇️
lib/internal/ffi-shared-buffer.js 98.87% <25.00%> (-0.18%) ⬇️

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

Comment thread doc/api/ffi.md
* `result`, `return`, or `returns` for the return type.
* `parameters` or `arguments` for the parameter type list.
* `result` {string} A [type name][type names] specifying the return type of the
function or callback. **Default:** `'void'`.
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.

If I can cast a vote for keeping return instead of result, I'd like to do so – that's just generally more in line with what the standardized terminology around function signatures is

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Well, if I have to pick one I'd go for return and arguments.
@Renegade334 Are you fine with doing so?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fine by me.

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

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. ffi Issues and PRs related to experimental Foreign Function Interface support. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants