Skip to content

Enable include files via package dependency #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 17, 2017

Conversation

jasongin
Copy link
Member

@jasongin jasongin commented Feb 28, 2017

This enables other packages to get the N-API headers via a package dependency, the same way that NAN works. The header files would be referenced like this in binding.gyp:

  'include_dirs': ["<!(node -p \"require('node-api').include\")"],

The same mechanism can be used in the future to enable referencing other things from the package like a static lib or maybe a gyp dependency, when we add the source code for back-compat.

Currently I've set a package name of node-api, with the expectation that we'll be able to acquire that package name.

@jasongin jasongin requested a review from boingoing March 17, 2017 19:38
@@ -0,0 +1,7 @@
const include = __dirname;
const isNodeApiBuiltin = process.versions.modules >= 52;

Choose a reason for hiding this comment

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

Is there a story for versioning N-API or do we always just provide whichever version is provided by the napi header/cc?

Copy link
Member Author

@jasongin jasongin Mar 17, 2017

Choose a reason for hiding this comment

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

For versions of node without N-API built-in, the N-API version will be the same as the node-api package version. For versions of node with N-API built-in, the N-API version will be the same as the node version. It's tricky because the former is determined at build time, while the latter is determined at run-time. This is an area we know we need to further define and document the behavior and best practices for native module developers.

Copy link

@boingoing boingoing left a comment

Choose a reason for hiding this comment

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

LGTM

@jasongin jasongin merged commit 2d7d8ee into nodejs:master Mar 17, 2017
@jasongin jasongin deleted the package branch March 17, 2017 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants