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

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

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 the node-api Issues and PRs related to the Node-API. label 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. v14.x labels Feb 12, 2021
@richardlau richardlau added the semver-minor PRs that contain new features and should be released in the next minor version. label Feb 12, 2021
@gabrielschulhof gabrielschulhof force-pushed the backport-node-api-module-file-name-to-v14.x branch from 866dd16 to dd3936e Compare February 12, 2021 03:07
@nodejs-github-bot
Copy link
Collaborator

@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-v14.x branch March 6, 2021 05:39
@gabrielschulhof gabrielschulhof restored the backport-node-api-module-file-name-to-v14.x branch March 7, 2021 03:59
@nodejs-github-bot
Copy link
Collaborator

@targos
Copy link
Member

targos commented Apr 25, 2021

This conflicts with #37728
Can you please rebase?

@gabrielschulhof
Copy link
Contributor Author

@targos rebased.

@gabrielschulhof
Copy link
Contributor Author

@danielleadams rebased.

@targos targos force-pushed the v14.x-staging branch 4 times, most recently from 49c8441 to 50b3bd0 Compare June 11, 2021 07:31
@richardlau richardlau force-pushed the v14.x-staging branch 2 times, most recently from 1f06fcf to 16dcd9c Compare July 5, 2021 16:02
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
Co-authored-by: Michael Dawson <mdawson@devrus.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
@targos targos force-pushed the backport-node-api-module-file-name-to-v14.x branch from 6b6f28e to db14845 Compare August 8, 2021 08:23
@targos targos added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 8, 2021
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 8, 2021
@nodejs-github-bot
Copy link
Collaborator

@targos
Copy link
Member

targos commented Aug 8, 2021

Landed in d98d193

@targos targos closed this Aug 8, 2021
targos pushed a commit that referenced this pull request Aug 8, 2021
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
Backport-PR-URL: #37327
Co-authored-by: Michael Dawson <mdawson@devrus.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
BethGriggs pushed a commit that referenced this pull request Aug 12, 2021
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
Backport-PR-URL: #37327
Co-authored-by: Michael Dawson <mdawson@devrus.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
MylesBorins pushed a commit that referenced this pull request Aug 31, 2021
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
Backport-PR-URL: #37327
Co-authored-by: Michael Dawson <mdawson@devrus.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
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#37327
Co-authored-by: Michael Dawson <mdawson@devrus.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
@gabrielschulhof gabrielschulhof deleted the backport-node-api-module-file-name-to-v14.x branch March 17, 2023 19:00
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.

6 participants