Skip to content

ffi: remove type aliases - #65409

Closed
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:remove-ffi-type-aliases
Closed

ffi: remove type aliases#65409
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:remove-ffi-type-aliases

Conversation

@trivikr

@trivikr trivikr commented Aug 20, 2026

Copy link
Copy Markdown
Member

Fixes: #64848

Remove duplicate FFI type spellings while the module is still experimental. Keep the canonical names exposed by ffi.types.


Assisted-by: codex:gpt-5.6-sol

Remove duplicate FFI type spellings while the module is still
experimental. Keep the canonical names exposed by `ffi.types`.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: codex:gpt-5.6-sol
@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++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Aug 20, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@trivikr trivikr added the ffi Issues and PRs related to experimental Foreign Function Interface support. label Aug 20, 2026
@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (f3a8f1a) to head (67b2ce5).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/ffi/fast.cc 93.75% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65409      +/-   ##
==========================================
+ Coverage   90.10%   90.12%   +0.01%     
==========================================
  Files         752      752              
  Lines      252207   252172      -35     
  Branches    47446    47439       -7     
==========================================
+ Hits       227254   227264      +10     
+ Misses      16261    16211      -50     
- Partials     8692     8697       +5     
Files with missing lines Coverage Δ
lib/ffi.js 94.73% <ø> (-0.05%) ⬇️
lib/internal/ffi-shared-buffer.js 54.28% <ø> (-1.30%) ⬇️
lib/internal/ffi/fast-api.js 94.34% <100.00%> (-0.13%) ⬇️
src/ffi/types.cc 55.51% <100.00%> (+1.69%) ⬆️
src/ffi/fast.cc 70.23% <93.75%> (+2.35%) ⬆️

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

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@ShogunPanda

Copy link
Copy Markdown
Contributor

I don't agree on this (or the initial issue).
People can just remember their most loved convention (i8 vs int8 and equivalent) but this way we are friendly with people coming from pretty much all languagues hierarchy.

@trivikr
trivikr marked this pull request as draft August 20, 2026 05:27
@trivikr trivikr added the blocked PRs that are blocked by other issues or PRs. label Aug 20, 2026
@trivikr

trivikr commented Aug 20, 2026

Copy link
Copy Markdown
Member Author

Converted to draft as we need to discuss the requirements first, and added blocked label.

@ShogunPanda Can you post your response on the issue #64848 so that the discussion can happen there?
It was opened three weeks ago and it has five upvotes which includes existing ffi contributors. That could mean existing ffi consumers prefer type aliases to be removed.

@ShogunPanda

Copy link
Copy Markdown
Contributor

Good point. Done.

@trivikr

trivikr commented Aug 20, 2026

Copy link
Copy Markdown
Member Author

Closing as we're highlighting canonical type names as preferred in #65417 as of now.

The removal can be revisited if community feels strongly about it in #65417

@trivikr trivikr closed this Aug 20, 2026
@trivikr
trivikr deleted the remove-ffi-type-aliases branch August 20, 2026 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocked PRs that are blocked by other issues or PRs. 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. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove FFI type aliases while it's still experimental

3 participants