Conversation
index.js
Outdated
| include: `"${__dirname}"`, // deprecated, can be removed as part of 4.0.0 | ||
| include_dir: includeDir, | ||
| gyp: path.join(includeDir, 'node_api.gyp:nothing'), | ||
| gyp: path.join(includeDir, 'node_addon_api.gyp'), |
There was a problem hiding this comment.
Is it all that harmful to leave this as-is and introduce a new key? If there's even a remote chance that it'll break folks, it may be simplest to not reuse this, but introduce a new key, and document what it does.
There was a problem hiding this comment.
Couldn't folks simply write the following?
...
'dependencies': [
"<!(node -p \"require('node-addon-api').include_dir\")/node_addon_api.gyp:node-addon-api-except"
]
...Then we would need only document that node_addon_api.gyp is the way to add node-addon-api to one's binding.gyp.
There was a problem hiding this comment.
It is unnecessary, but having a single gyp entry point would be good. And using the include_dir works -- nothing is wrong. It is simpler to have a gyp file short name, like
'dependencies': [
"<!(node -p \"require('node-addon-api').include_dir\")/node_addon_api.gyp:node-addon-api-except"
]
// versus...
'dependencies': [
"<!(node -p \"require('node-addon-api').gyp\"):node-addon-api-except"
]
Would require('node-addon-api').targets sound good to you? For instance,
'dependencies': [
"<!(node -p \"require('node-addon-api').targets\"):node-addon-api-except"
]
There was a problem hiding this comment.
I've updated the PR to name the entrypoint as require('node-addon-api').targets.
1bbdeea to
bd2d790
Compare
Fixes: #1379
As documented at https://gyp.gsrc.io/docs/InputFormatReference.md#processing-order, a dependency's setting is processed after merging the root target_defaults, and variable expansion is allowed in dependency specifiers. This allows addons to include node-addon-api and common configs with a single line of gyp config. For example, when the addon needs c++ exception feature, they can simply declare the dependency on
node-addon-api-except:No need to copy a paragraph of configs into addons' binding.gyp like documented at https://github.com/nodejs/node-addon-api/blob/9aaf3b1/doc/setup.md#installation-and-usage anymore.