Skip to content

node-api: Loosen restriction on the type of value that we can create a ref on#39926

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.

Sign up for GitHub

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

Jump to bottom
Closed
JckXia wants to merge 2 commits into from
Closed

node-api: Loosen restriction on the type of value that we can create a ref on #39926

JckXia wants to merge 2 commits into from

Conversation

@JckXia
Copy link
Member

@JckXiaJckXia commented Aug 29, 2021

In a recent discussion in NAPI regarding handling exceptions thrown by Javascript code that are not objects (nodejs/node-addon-api#912) , we've discovered that unlike with other primitives (numbers, string, etc), symbol values cannot be retrieved. To deal with this issue we've decided to loosen the restriction on the type of values that napi_create_reference can receive. (At the moment only function and object types are allowed)

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Aug 29, 2021
@JckXiaJckXia changed the title Loosen restriction on the type of value that we can create a reference on using napi_create_referenceLoosen restriction on the type of value that we can create a ref onAug 29, 2021
@JckXiaJckXia changed the title Loosen restriction on the type of value that we can create a ref onnode-api: Loosen restriction on the type of value that we can create a ref onAug 29, 2021
@JckXiaJckXiaforce-pushed the loosen-napi-ref-restriction branch from 25dcc0e to 1ba9a54CompareAugust 29, 2021 15:05
mhdawson
mhdawson reviewed Sep 3, 2021
View reviewed changes
@mhdawsonmhdawson added the node-api Issues and PRs related to the Node-API. label Sep 3, 2021
mhdawson
mhdawson approved these changes Sep 3, 2021
View reviewed changes
Copy link
Member

@mhdawsonmhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@mhdawson
Copy link
Member

mhdawson commented Sep 3, 2021

Believe error reported on commit message is due to additional comment to incorporate comment which will be squashed.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 3, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/39766/

@mhdawsonmhdawson added the semver-minor PRs that contain new features and should be released in the next minor version. label Sep 3, 2021
@JckXiaJckXiaforce-pushed the loosen-napi-ref-restriction branch from 64732c8 to 0bb19e3CompareSeptember 5, 2021 22:48
@JckXia
Copy link
MemberAuthor

JckXia commented Sep 5, 2021

Updated commit message to adhere to format. (adding a test prefix)

@JckXiaJckXiaforce-pushed the loosen-napi-ref-restriction branch from 0bb19e3 to 3b6a698CompareSeptember 6, 2021 15:12
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 7, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/39858/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 8, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/39860/

mhdawson pushed a commit that referenced this pull request Sep 8, 2021
@JckXia@mhdawson
src: make napi_create_reference accept symbol
33b5107
PR-URL: #39926 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: James M Snell <[email protected]>
@mhdawson
Copy link
Member

mhdawson commented Sep 8, 2021•
edited
Loading

Landed in 33b5107
Thanks @JckXia!

@mhdawsonmhdawson closed this Sep 8, 2021
This was referenced Sep 14, 2021
Open
Open
Open
BethGriggs pushed a commit that referenced this pull request Sep 21, 2021
@JckXia@BethGriggs
src: make napi_create_reference accept symbol
fe920b6
PR-URL: #39926 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: James M Snell <[email protected]>
BethGriggs added a commit that referenced this pull request Sep 21, 2021
@BethGriggs
2021-09-22, Version 16.10.0 (Current)
45a042b
Notable changes: crypto: * (SEMVER-MINOR) add rsa-pss keygen parameters (Filip Skokan) #39927 doc: * add Ayase-252 to collaborators (Qingyu Deng) #40078 fs: * (SEMVER-MINOR) make `open` and `close` stream override optional when unused (Antoine du Hamel) #40013 http: * (SEMVER-MINOR) limit requests per connection (Artur K) #40082 src: * (SEMVER-MINOR) add --no-global-search-paths cli option (Cheng Zhao) #39754 * (SEMVER-MINOR) add option to disable global search paths (Cheng Zhao) #39754 * (SEMVER-MINOR) make napi_create_reference accept symbol (JckXia) #39926 stream: * (SEMVER-MINOR) add signal support to pipeline generators (Robert Nagy) #39067 PR-URL: TODO
BethGriggs added a commit that referenced this pull request Sep 21, 2021
@BethGriggs
2021-09-22, Version 16.10.0 (Current)
10b4fe1
Notable changes: crypto: * (SEMVER-MINOR) add rsa-pss keygen parameters (Filip Skokan) #39927 doc: * add Ayase-252 to collaborators (Qingyu Deng) #40078 fs: * (SEMVER-MINOR) make `open` and `close` stream override optional when unused (Antoine du Hamel) #40013 http: * (SEMVER-MINOR) limit requests per connection (Artur K) #40082 src: * (SEMVER-MINOR) add --no-global-search-paths cli option (Cheng Zhao) #39754 * (SEMVER-MINOR) add option to disable global search paths (Cheng Zhao) #39754 * (SEMVER-MINOR) make napi_create_reference accept symbol (JckXia) #39926 stream: * (SEMVER-MINOR) add signal support to pipeline generators (Robert Nagy) #39067 PR-URL: TODO
@BethGriggsBethGriggs mentioned this pull request Sep 21, 2021
Merged
1 task
BethGriggs added a commit that referenced this pull request Sep 22, 2021
@BethGriggs
2021-09-22, Version 16.10.0 (Current)
aff2a0a
Notable changes: crypto: * (SEMVER-MINOR) add rsa-pss keygen parameters (Filip Skokan) #39927 doc: * add Ayase-252 to collaborators (Qingyu Deng) #40078 fs: * (SEMVER-MINOR) make `open` and `close` stream override optional when unused (Antoine du Hamel) #40013 http: * (SEMVER-MINOR) limit requests per connection (Artur K) #40082 src: * (SEMVER-MINOR) add --no-global-search-paths cli option (Cheng Zhao) #39754 * (SEMVER-MINOR) add option to disable global search paths (Cheng Zhao) #39754 * (SEMVER-MINOR) make napi_create_reference accept symbol (JckXia) #39926 stream: * (SEMVER-MINOR) add signal support to pipeline generators (Robert Nagy) #39067 PR-URL: #40175
BethGriggs added a commit that referenced this pull request Sep 22, 2021
@BethGriggs
2021-09-22, Version 16.10.0 (Current)
8d5675e
Notable changes: crypto: * (SEMVER-MINOR) add rsa-pss keygen parameters (Filip Skokan) #39927 doc: * add Ayase-252 to collaborators (Qingyu Deng) #40078 fs: * (SEMVER-MINOR) make `open` and `close` stream override optional when unused (Antoine du Hamel) #40013 http: * (SEMVER-MINOR) limit requests per connection (Artur K) #40082 src: * (SEMVER-MINOR) add --no-global-search-paths cli option (Cheng Zhao) #39754 * (SEMVER-MINOR) add option to disable global search paths (Cheng Zhao) #39754 * (SEMVER-MINOR) make napi_create_reference accept symbol (JckXia) #39926 stream: * (SEMVER-MINOR) add signal support to pipeline generators (Robert Nagy) #39067 PR-URL: #40175
@gabrielschulhofgabrielschulhof mentioned this pull request Dec 17, 2021
Closed
richardlau pushed a commit that referenced this pull request Jan 25, 2022
@JckXia@richardlau
src: make napi_create_reference accept symbol
f74fe2a
PR-URL: #39926 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: James M Snell <[email protected]>
richardlau added a commit that referenced this pull request Jan 25, 2022
@richardlau
2022-02-01, Version 14.19.0 'Fermium' (LTS)
9beb4f8
Notable changes: Corepack: Node.js now includes Corepack, a script that acts as a bridge between Node.js projects and the package managers they are intended to be used with during development. In practical terms, Corepack will let you use Yarn and pnpm without having to install them - just like what currently happens with npm, which is shipped in Node.js by default. Contributed by Maël Nison - #39608 ICU updated: ICU has been updated to 70.1. This updates timezone database to 2021a3, including bringing forward the start for DST for Jordan from March to February. Contributed by Michaël Zasso - #40658 New option to disable loading of native addons: A new command line option `--no-addons` has been added to disallow loading of native addons. Contributed by Dominic Elm - #39977 Updated Root Certificates: Root certificates have been updated to those from Mozilla's Network Security Services 3.71. Contributed by Richard Lau - #40280 Other Notable Changes: crypto: * (SEMVER-MINOR) make FIPS related options always available (Vít Ondruch) #36341 lib: * (SEMVER-MINOR) add unsubscribe method to non-active DC channels (simon-id) #40433 * (SEMVER-MINOR) add return value for DC channel.unsubscribe (simon-id) #40433 module: * (SEMVER-MINOR) support pattern trailers (Guy Bedford) #39635 src: * (SEMVER-MINOR) make napi_create_reference accept symbol (JckXia) #39926 PR-URL: #41696
@richardlaurichardlau mentioned this pull request Jan 25, 2022
Merged
richardlau added a commit that referenced this pull request Feb 1, 2022
@richardlau
2022-02-01, Version 14.19.0 'Fermium' (LTS)
1c23c1e
Notable changes: Corepack: Node.js now includes Corepack, a script that acts as a bridge between Node.js projects and the package managers they are intended to be used with during development. In practical terms, Corepack will let you use Yarn and pnpm without having to install them - just like what currently happens with npm, which is shipped in Node.js by default. Contributed by Maël Nison - #39608 ICU updated: ICU has been updated to 70.1. This updates timezone database to 2021a3, including bringing forward the start for DST for Jordan from March to February. Contributed by Michaël Zasso - #40658 New option to disable loading of native addons: A new command line option `--no-addons` has been added to disallow loading of native addons. Contributed by Dominic Elm - #39977 Updated Root Certificates: Root certificates have been updated to those from Mozilla's Network Security Services 3.71. Contributed by Richard Lau - #40280 Other Notable Changes: crypto: * (SEMVER-MINOR) make FIPS related options always available (Vít Ondruch) #36341 lib: * (SEMVER-MINOR) add unsubscribe method to non-active DC channels (simon-id) #40433 * (SEMVER-MINOR) add return value for DC channel.unsubscribe (simon-id) #40433 module: * (SEMVER-MINOR) support pattern trailers (Guy Bedford) #39635 src: * (SEMVER-MINOR) make napi_create_reference accept symbol (JckXia) #39926 PR-URL: #41696
mwalbeck pushed a commit to mwalbeck/docker-jellyfin-livestream that referenced this pull request Mar 16, 2022
@mwalbeck
Update Node.js to v14.19.0 (#97)
7fa000e
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [node](https://github.com/nodejs/node) | stage | minor | `14.18.3-bullseye-slim` -> `14.19.0-bullseye-slim` | --- ### Release Notes <details> <summary>nodejs/node</summary> ### [`v14.19.0`](https://github.com/nodejs/node/releases/v14.19.0) [Compare Source](nodejs/node@v14.18.3...v14.19.0) ##### Notable Changes ##### Corepack Node.js now includes Corepack, a script that acts as a bridge between Node.js projects and the package managers they are intended to be used with during development. In practical terms, **Corepack will let you use Yarn and pnpm without having to install them** - just like what currently happens with npm, which is shipped in Node.js by default. Please head over to the [Corepack documentation page](https://nodejs.org/dist/latest-v14.x/docs/api/corepack.html) for more information on how to use it. Contributed by Maël Nison - [#&#8203;39608](nodejs/node#39608) ##### ICU updated ICU has been updated to 70.1. This updates timezone database to 2021a3, including bringing forward the start for DST for Jordan from March to February. Contributed by Michaël Zasso - [#&#8203;40658](nodejs/node#40658) ##### New option to disable loading of native addons A new command line option `--no-addons` has been added to disallow loading of native addons. Contributed by Dominic Elm - [#&#8203;39977](nodejs/node#39977) ##### Updated Root Certificates Root certificates have been updated to those from Mozilla's Network Security Services 3.71. Contributed by Richard Lau - [#&#8203;40280](nodejs/node#40280) ##### Other Notable Changes - \[[`0d448eaab5`](nodejs/node@0d448eaab5)] - **(SEMVER-MINOR)** **crypto**: make FIPS related options always available (Vít Ondruch) [#&#8203;36341](nodejs/node#36341) - \[[`004eafbebf`](nodejs/node@004eafbebf)] - **(SEMVER-MINOR)** **lib**: add unsubscribe method to non-active DC channels (simon-id) [#&#8203;40433](nodejs/node#40433) - \[[`625be7585d`](nodejs/node@625be7585d)] - **(SEMVER-MINOR)** **lib**: add return value for DC channel.unsubscribe (simon-id) [#&#8203;40433](nodejs/node#40433) - \[[`607bc74eae`](nodejs/node@607bc74eae)] - **(SEMVER-MINOR)** **module**: support pattern trailers (Guy Bedford) [#&#8203;39635](nodejs/node#39635) - \[[`f74fe2a59c`](nodejs/node@f74fe2a59c)] - **(SEMVER-MINOR)** **src**: make napi_create_reference accept symbol (JckXia) [#&#8203;39926](nodejs/node#39926) ##### Commits - \[[`0231ffa501`](nodejs/node@0231ffa501)] - **build**: add `--without-corepack` (Jonah Snider) [#&#8203;41060](nodejs/node#41060) - \[[`5389b8ab05`](nodejs/node@5389b8ab05)] - **crypto**: update root certificates (Richard Lau) [#&#8203;40280](nodejs/node#40280) - \[[`0d448eaab5`](nodejs/node@0d448eaab5)] - **(SEMVER-MINOR)** **crypto**: make FIPS related options always available (Vít Ondruch) [#&#8203;36341](nodejs/node#36341) - \[[`cd20ecc7cb`](nodejs/node@cd20ecc7cb)] - **deps**: upgrade Corepack to 0.10 (Maël Nison) [#&#8203;40374](nodejs/node#40374) - \[[`737df75e17`](nodejs/node@737df75e17)] - **(SEMVER-MINOR)** **deps**: add corepack (Maël Nison) [#&#8203;39608](nodejs/node#39608) - \[[`b85aa5a143`](nodejs/node@b85aa5a143)] - **deps**: upgrade npm to 6.14.16 (Ruy Adorno) [#&#8203;41603](nodejs/node#41603) - \[[`2755d391a5`](nodejs/node@2755d391a5)] - **deps**: update ICU to 70.1 (Michaël Zasso) [#&#8203;40658](nodejs/node#40658) - \[[`3089326d89`](nodejs/node@3089326d89)] - **deps**: update archs files for OpenSSL-1.1.1m (Richard Lau) [#&#8203;41173](nodejs/node#41173) - \[[`59da7c12aa`](nodejs/node@59da7c12aa)] - **deps**: upgrade openssl sources to 1.1.1m (Richard Lau) [#&#8203;41173](nodejs/node#41173) - \[[`cede1f26f6`](nodejs/node@cede1f26f6)] - **deps**: add -fno-strict-aliasing flag to libuv (Daniel Bevenius) [#&#8203;40631](nodejs/node#40631) - \[[`4477da858f`](nodejs/node@4477da858f)] - **doc**: fix corepack grammar for `--force` flag (Steven) [#&#8203;40762](nodejs/node#40762) - \[[`5971d58600`](nodejs/node@5971d58600)] - **doc**: add missing YAML tag in `esm.md` (Antoine du Hamel) [#&#8203;41516](nodejs/node#41516) - \[[`e903798ae1`](nodejs/node@e903798ae1)] - **doc**: add note regarding unfinished TLA (Antoine du Hamel) [#&#8203;41434](nodejs/node#41434) - \[[`a90defebcf`](nodejs/node@a90defebcf)] - **esm**: make `process.exit()` default to exit code 0 (Gang Chen) [#&#8203;41388](nodejs/node#41388) - \[[`fc328f1ab0`](nodejs/node@fc328f1ab0)] - **fs**: nullish coalescing to respect zero positional reads (Omar El-Mihilmy) [#&#8203;40716](nodejs/node#40716) - \[[`004eafbebf`](nodejs/node@004eafbebf)] - **(SEMVER-MINOR)** **lib**: add unsubscribe method to non-active DC channels (simon-id) [#&#8203;40433](nodejs/node#40433) - \[[`625be7585d`](nodejs/node@625be7585d)] - **(SEMVER-MINOR)** **lib**: add return value for DC channel.unsubscribe (simon-id) [#&#8203;40433](nodejs/node#40433) - \[[`2c365961d0`](nodejs/node@2c365961d0)] - **module**: support pattern trailers for imports field (Guy Bedford) [#&#8203;40041](nodejs/node#40041) - \[[`607bc74eae`](nodejs/node@607bc74eae)] - **(SEMVER-MINOR)** **module**: support pattern trailers (Guy Bedford) [#&#8203;39635](nodejs/node#39635) - \[[`f74fe2a59c`](nodejs/node@f74fe2a59c)] - **(SEMVER-MINOR)** **src**: make napi_create_reference accept symbol (JckXia) [#&#8203;39926](nodejs/node#39926) - \[[`b050c65885`](nodejs/node@b050c65885)] - **src**: add option to disable loading native addons (Dominic Elm) [#&#8203;39977](nodejs/node#39977) - \[[`c1695ac68a`](nodejs/node@c1695ac68a)] - **tools**: update certdata.txt (Richard Lau) [#&#8203;40280](nodejs/node#40280) </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Reviewed-on: https://git.walbeck.it/mwalbeck/docker-jellyfin-livestream/pulls/97 Co-authored-by: renovate-bot <[email protected]> Co-committed-by: renovate-bot <[email protected]>
@toyobayashitoyobayashi mentioned this pull request Mar 29, 2023
Merged
@toyobayashitoyobayashi mentioned this pull request Apr 26, 2023
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnelljasnelljasnell approved these changes

@mhdawsonmhdawsonmhdawson approved these changes

Assignees

No one assigned

Labels

c++Issues and PRs that require attention from people who are familiar with C++.needs-ciPRs that need a full CI run.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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@JckXia@mhdawson@nodejs-github-bot@jasnell@richardlau