Skip to content
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

[v12.x] node-api: allow retrieval of add-on file name #37328

Conversation

gabrielschulhof
Copy link
Contributor

Unlike JS-only modules, native add-ons are always associated with a
dynamic shared object from which they are loaded. Being able to
retrieve its absolute path is important to native-only add-ons, i.e.
add-ons that are not themselves being loaded from a JS-only module
located in the same package as the native add-on itself.

Currently, the file name is obtained at environment construction time
from the JS module.filename. Nevertheless, the presence of module
is not required, because the file name could also be passed in via a
private property added onto exports from the process.dlopen
binding.

As an attempt at future-proofing, the file name is provided as a URL,
i.e. prefixed with the file:// protocol.

Fixes: nodejs/node-addon-api#449
PR-URL: #37195
Co-authored-by: @mhdawson
Reviewed-By: @mhdawson

@gabrielschulhof gabrielschulhof added semver-minor PRs that contain new features and should be released in the next minor version. node-api Issues and PRs related to the Node-API. labels Feb 12, 2021
@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. v12.x labels Feb 12, 2021
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 14, 2021

@gabrielschulhof
Copy link
Contributor Author

The solution to the conflict in this backport backport consists of restoring the names of the macros used in the test:

Whereas on the main branch it was

  • NODE_API_CALL, and
  • DECLARE_NODE_API_GETTER,

on this branch it is the old

  • NAPI_CALL, and
  • DECLARE_NAPI_GETTER

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@gabrielschulhof gabrielschulhof deleted the backport-node-api-module-file-name-to-v12.x branch March 6, 2021 05:39
@gabrielschulhof gabrielschulhof restored the backport-node-api-module-file-name-to-v12.x branch March 7, 2021 03:57
Unlike JS-only modules, native add-ons are always associated with a
dynamic shared object from which they are loaded. Being able to
retrieve its absolute path is important to native-only add-ons, i.e.
add-ons that are not themselves being loaded from a JS-only module
located in the same package as the native add-on itself.

Currently, the file name is obtained at environment construction time
from the JS `module.filename`. Nevertheless, the presence of `module`
is not required, because the file name could also be passed in via a
private property added onto `exports` from the `process.dlopen`
binding.

As an attempt at future-proofing, the file name is provided as a URL,
i.e. prefixed with the `file://` protocol.

Fixes: nodejs/node-addon-api#449
PR-URL: nodejs#37195
Backport-PR-URL: nodejs#37328
Co-authored-by: Michael Dawson <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@richardlau richardlau force-pushed the backport-node-api-module-file-name-to-v12.x branch from 6666b73 to f569209 Compare March 17, 2021 21:12
@richardlau
Copy link
Member

Landed in f569209.

@richardlau richardlau merged commit f569209 into nodejs:v12.x-staging Mar 17, 2021
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++. lib / src Issues and PRs related to general changes in the lib or src directory. node-api Issues and PRs related to the Node-API. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants