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

[v10.x] n-api: backport ArrayBuffer detaching APIs #33061

Closed

Conversation

legendecas
Copy link
Member

@legendecas legendecas commented Apr 25, 2020

This PR backports ArrayBuffer detaching related N-API to v10.x, commits:

n-api: add napi_detach_arraybuffer

As ArrayBuffer#detach is an ecma spec operation
(Section 24.1.1.3),
it might be good to have it in N-API.

Fixes #29674

PR-URL: #29768
Reviewed-By: Anna Henningsen anna@addaleax.net
Reviewed-By: David Carlier devnexen@gmail.com
Reviewed-By: Colin Ihrig cjihrig@gmail.com
Reviewed-By: James M Snell jasnell@gmail.com
Reviewed-By: Gabriel Schulhof gabriel.schulhof@intel.com

n-api: implement napi_is_detached_arraybuffer

This implements ArrayBuffer#IsDetachedBuffer operation as per ECMAScript
specification Section 24.1.1.2 https://tc39.es/ecma262/#sec-isdetachedbuffer

Closes: #29955

PR-URL: #30613
Fixes: #29955
Reviewed-By: Anna Henningsen anna@addaleax.net
Reviewed-By: Gus Caplan me@gus.host
Reviewed-By: David Carlier devnexen@gmail.com
Reviewed-By: Chengzhong Wu legendecas@gmail.com
Reviewed-By: Michael Dawson michael_dawson@ca.ibm.com

doc,n-api: mark napi_detach_arraybuffer as experimental

As its actual release stage.

PR-URL: #30703
Reviewed-By: Denys Otrishko shishugi@gmail.com
Reviewed-By: Luigi Pinca luigipinca@gmail.com
Reviewed-By: Michael Dawson michael_dawson@ca.ibm.com
Reviewed-By: Tobias Nießen tniessen@tnie.de

@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. v10.x labels Apr 25, 2020
@legendecas legendecas changed the title [10.x] n-api: add napi_detach_arraybuffer [v10.x] n-api: add napi_detach_arraybuffer Apr 25, 2020
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@legendecas legendecas added the node-api Issues and PRs related to the Node-API. label Apr 27, 2020
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@legendecas
Copy link
Member Author

/cc @nodejs/releasers would this be possible to be able to release in the next v10.x minor release? As far as I can tell, v10.x will begin maintenance state from 2020-05-19. I'm wondering if there will be a minor release before that?

src/node_api.cc Outdated Show resolved Hide resolved
@gabrielschulhof gabrielschulhof added the semver-minor PRs that contain new features and should be released in the next minor version. label Apr 29, 2020
@legendecas legendecas changed the title [v10.x] n-api: add napi_detach_arraybuffer [v10.x] n-api: backport ArrayBuffer detaching APIs May 2, 2020
@legendecas
Copy link
Member Author

@gabrielschulhof updated :). Since napi_is_detached_arraybuffer may be useful with napi_detach_arraybuffer, I've backported it too along with the PR.

@nodejs-github-bot
Copy link
Collaborator

@legendecas legendecas requested a review from lundibundi May 2, 2020 12:34
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@richardlau
Copy link
Member

@legendecas Please could you rebase this onto the current v10.x-staging branch?

legendecas and others added 3 commits July 2, 2020 00:28
As ArrayBuffer#detach is an ecma spec operation
([Section 24.1.1.3](https://tc39.es/ecma262/#sec-detacharraybuffer)),
it might be good to have it in N-API.

Fixes nodejs#29674

PR-URL: nodejs#29768
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
This implements ArrayBuffer#IsDetachedBuffer operation as per ECMAScript
specification Section 24.1.1.2 https://tc39.es/ecma262/#sec-isdetachedbuffer

Closes: nodejs#29955

PR-URL: nodejs#30613
Fixes: nodejs#29955
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
As its actual release stage.

PR-URL: nodejs#30703
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 1, 2020

richardlau pushed a commit that referenced this pull request Jul 1, 2020
As ArrayBuffer#detach is an ecma spec operation
([Section 24.1.1.3](https://tc39.es/ecma262/#sec-detacharraybuffer)),
it might be good to have it in N-API.

Fixes: #29674
PR-URL: #29768
Backport-PR-URL: #33061
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
richardlau pushed a commit that referenced this pull request Jul 1, 2020
This implements ArrayBuffer#IsDetachedBuffer operation as per ECMAScript
specification Section 24.1.1.2 https://tc39.es/ecma262/#sec-isdetachedbuffer

Closes: #29955

PR-URL: #30613
Backport-PR-URL: #33061
Fixes: #29955
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
richardlau pushed a commit that referenced this pull request Jul 1, 2020
As its actual release stage.

PR-URL: #30703
Backport-PR-URL: #33061
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@richardlau
Copy link
Member

Landed in 3dbd8cd...5dab101

@richardlau richardlau closed this Jul 1, 2020
@legendecas legendecas deleted the backport-29768-to-10 branch July 2, 2020 03:44
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.

8 participants