src: include node_api_types.h instead of node_api.h in node.h
#60496
+2
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Review requested:
|
nodejs-github-bot
added
c++
addaleax
added
semver-major
juanarbol
approved these changes
Oct 30, 2025
addaleax
commented
Oct 30, 2025
| "Experimental features may be unstable." | ||
| #endif | ||
| #endif | ||
|
|
There was a problem hiding this comment.
For context, this was moved because this file actually depends on these macros being applied, so previously there was an implicit requirement that js_native_api.h would need to be included prior to this file, either directly or indirectly.
There was a problem hiding this comment.
I believe this is not a semver-major change. I'm wondering if this can be split into a PR so that it can be backport-ed? Usually we don't mark node-api header changes as semver-major and node-api headers are sync-ed across LTS versions.
There was a problem hiding this comment.
@legendecas Yup, that makes a ton of sense. Opened #60512 with the non-breaking changes to Node-API headers.
github-actions
bot
removed
the
request-ci
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #60496 +/- ##
==========================================
+ Coverage 88.54% 88.55% +0.01%
==========================================
Files 704 704
Lines 207843 207843
Branches 40044 40041 -3
==========================================
+ Hits 184028 184057 +29
+ Misses 15858 15834 -24
+ Partials 7957 7952 -5
🚀 New features to boost your workflow:
|
addaleax
added a commit
to addaleax/node
that referenced
this pull request
Oct 31, 2025
This means that `node.h` can include only this file, instead of the entirety of `node_api.h`. Split out from nodejs#60496 since it was rightfully pointed out that the breaking part of the change should not touch Node-API headers.
nodejs-github-bot
pushed a commit
that referenced
this pull request
Nov 2, 2025
This means that `node.h` can include only this file, instead of the entirety of `node_api.h`. Split out from #60496 since it was rightfully pointed out that the breaking part of the change should not touch Node-API headers. PR-URL: #60512 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
|
Sorry for being late. This needs a rebase after #60512 landed. |
Including `node.h` should not result in all of Node-API also being available to callers. Users who want `node_api.h` contents should explicitly include that header. We currently include it specifically for `napi_addon_register_func`; by moving that into `node_api_types.h` and including that instead, we can reduce unintentionally included API surface a lot. Refs: nodejs#60345 (comment)
addaleax
force-pushed
the
node-h-only-include-node-api-types.h
branch
from
617271c to
73d929d
Compare
addaleax
added
review wanted
|
@legendecas Yep, done! And no worries at all, your input here has been great. |
legendecas
approved these changes
Nov 2, 2025
github-actions
bot
removed
the
request-ci
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Edit: Split out #60512 from this PR and marking this PR as blocked on it
Including
node.hshould not result in all of Node-API also being available to callers. Users who wantnode_api.hcontents should explicitly include that header.We currently include it specifically for
napi_addon_register_func; by moving that intonode_api_types.hand including that instead, we can reduce unintentionally included API surface a lot.Refs: #60345 (comment)