Skip to content

[v10.x]: Backport instance data - #30537

Closed
gabrielschulhof wants to merge 2 commits into
nodejs:v10.x-stagingfrom
gabrielschulhof:backport-instance-data
Closed

[v10.x]: Backport instance data#30537
gabrielschulhof wants to merge 2 commits into
nodejs:v10.x-stagingfrom
gabrielschulhof:backport-instance-data

Conversation

@gabrielschulhof

@gabrielschulhofgabrielschulhof commented Nov 18, 2019

Copy link
Copy Markdown
Contributor
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@gabrielschulhofgabrielschulhof added the node-api Issues and PRs related to the Node-API. label Nov 18, 2019
@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. v10.x labels Nov 18, 2019
@gabrielschulhof

Copy link
Copy Markdown
ContributorAuthor

Needed for nodejs/node-addon-api#567

@gabrielschulhofgabrielschulhof added the semver-minor PRs that contain new features and should be released in the next minor version. label Nov 18, 2019
@gabrielschulhofgabrielschulhof changed the title v10.x: Backport instance data[v10.x]: Backport instance dataNov 19, 2019
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@mhdawson

Copy link
Copy Markdown
Member

@gabrielf I assume the commit did not apply cleanly. Can you identify the parts you had to update in order to limit what we need to review?

@gabrielschulhof

Copy link
Copy Markdown
ContributorAuthor

@mhdawson the separation of js_native_api from node_api is not present in this version, so backporting is not straight-forward. The napi_env in this version is not broken into node_napi_env and napi_env. So, the parts modifying napi_env are the most relevant. Yet the places from which NapiCallIntoModule is being performed had to be updated manually as well, because the files were not separated into node_api and js_native_api, like on master.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@gabrielschulhof

Copy link
Copy Markdown
ContributorAuthor

Rebased.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@gabrielschulhof

Copy link
Copy Markdown
ContributorAuthor

Rebased again.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@gabrielschulhof

Copy link
Copy Markdown
ContributorAuthor

Rebased again.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

addaleaxand others added 2 commits December 18, 2019 11:46
These do not need to be macros.
PR-URL: nodejs#26128
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Adds `napi_set_instance_data()` and `napi_get_instance_data()`, which
allow native addons to store their data on and retrieve their data from
`napi_env`. `napi_set_instance_data()` accepts a finalizer which is
called when the `node::Environment()` is destroyed.
This entails rendering the `napi_env` local to each add-on.
Fixes: nodejs/abi-stable-node#378
PR-URL: nodejs#28682
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@gabrielschulhof

Copy link
Copy Markdown
ContributorAuthor

@BethGriggs can we get this into v10.x before it goes into maintenance?

@BethGriggs

Copy link
Copy Markdown
Member

@gabrielschulhof, yes - we're aiming to have one more semver-minor release in Q1 2020 (probably late Jan/Feb time), I'll land this commit in staging once v10.18.1 has gone out - #30796

@gabrielschulhofgabrielschulhof mentioned this pull request Jan 8, 2020
@gabrielschulhof

Copy link
Copy Markdown
ContributorAuthor

@BethGriggs is this still on track to land?

@BethGriggs

BethGriggs commented Feb 19, 2020

Copy link
Copy Markdown
Member

@gabrielschulhof yes, but I'm unsure on the timing of the release (nodejs/Release#504 needs updating).

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@BethGriggs

Copy link
Copy Markdown
Member

Possibly will not get a green CI on this until #31887 lands (@AshCripps and I are currently working on backporting the appropriate flaky test markers from v12.x)

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

nodejs-github-bot commented Feb 25, 2020

Copy link
Copy Markdown
Collaborator

BethGriggs pushed a commit that referenced this pull request Feb 25, 2020
These do not need to be macros.
PR-URL: #26128
Backport-PR-URL: #30537
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this pull request Feb 25, 2020
Adds `napi_set_instance_data()` and `napi_get_instance_data()`, which
allow native addons to store their data on and retrieve their data from
`napi_env`. `napi_set_instance_data()` accepts a finalizer which is
called when the `node::Environment()` is destroyed.
This entails rendering the `napi_env` local to each add-on.
Fixes: nodejs/abi-stable-node#378
PR-URL: #28682
Backport-PR-URL: #30537
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@BethGriggs

Copy link
Copy Markdown
Member

Landed in 3f9cec3...f29fb14

@gabrielschulhof
gabrielschulhof deleted the backport-instance-data branch April 21, 2020 02:58
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@mhdawson@BethGriggs@gengjiawen@addaleax