Skip to content

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

Closed
gabrielschulhof wants to merge 1 commit into
nodejs:v14.x-stagingfrom
gabrielschulhof:backport-node-api-module-file-name-to-v14.x
Closed

[v14.x] node-api: allow retrieval of add-on file name#37327
gabrielschulhof wants to merge 1 commit into
nodejs:v14.x-stagingfrom
gabrielschulhof:backport-node-api-module-file-name-to-v14.x

Conversation

@gabrielschulhof

Copy link
Copy Markdown
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

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

Copy link
Copy Markdown
Collaborator

@gabrielschulhof

Copy link
Copy Markdown
ContributorAuthor

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

@mhdawsonmhdawson left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
Collaborator

@targos

Copy link
Copy Markdown
Member

This conflicts with #37728
Can you please rebase?

@gabrielschulhof
gabrielschulhofforce-pushed the backport-node-api-module-file-name-to-v14.x branch from dd3936e to 6b6f28eCompareJune 4, 2021 14:19
@gabrielschulhof

Copy link
Copy Markdown
ContributorAuthor

@targos rebased.

@gabrielschulhof

Copy link
Copy Markdown
ContributorAuthor

@danielleadams rebased.

@targos
targosforce-pushed the v14.x-staging branch 4 times, most recently from 49c8441 to 50b3bd0CompareJune 11, 2021 07:31
@richardlau
richardlauforce-pushed the v14.x-staging branch 2 times, most recently from 1f06fcf to 16dcd9cCompareJuly 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
targosforce-pushed the backport-node-api-module-file-name-to-v14.x branch from 6b6f28e to db14845CompareAugust 8, 2021 08:23
@targostargos added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 8, 2021
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 8, 2021
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@targos

Copy link
Copy Markdown
Member

Landed in d98d193

@targostargos 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>
@gabrielschulhof
gabrielschulhof deleted the backport-node-api-module-file-name-to-v14.x branch March 17, 2023 19:00
Sign up for freeto 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 / srcIssues and PRs related to general changes in the lib or src directory.node-apiIssues and PRs related to the Node-API.semver-minorPRs 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

@gabrielschulhof@nodejs-github-bot@targos@legendecas@mhdawson@richardlau