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.

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

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

Closed
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

wants to merge 2 commits into from

Conversation

JckXia
Copy link
Member

@JckXia JckXia 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-bot nodejs-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
@JckXia JckXia changed the title Loosen restriction on the type of value that we can create a reference on using napi_create_reference Loosen restriction on the type of value that we can create a ref on Aug 29, 2021
@JckXia JckXia changed the title Loosen restriction on the type of value that we can create a ref on node-api: Loosen restriction on the type of value that we can create a ref on Aug 29, 2021
@JckXia JckXia force-pushed the loosen-napi-ref-restriction branch from 25dcc0e to 1ba9a54 Compare August 29, 2021 15:05
@mhdawson mhdawson 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

@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

@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

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

@mhdawson mhdawson added the semver-minor PRs that contain new features and should be released in the next minor version. label Sep 3, 2021
@JckXia JckXia force-pushed the loosen-napi-ref-restriction branch from 64732c8 to 0bb19e3 Compare September 5, 2021 22:48
@JckXia
Copy link
Member Author

JckXia commented Sep 5, 2021

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

@JckXia JckXia force-pushed the loosen-napi-ref-restriction branch from 0bb19e3 to 3b6a698 Compare September 6, 2021 15:12
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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 <midawson@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@mhdawson
Copy link
Member

mhdawson commented Sep 8, 2021 •
edited
Loading

Landed in 33b5107
Thanks @JckXia!

@mhdawson mhdawson 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 <midawson@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
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
@BethGriggs BethGriggs 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
@gabrielschulhof gabrielschulhof 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 <midawson@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
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
@richardlau richardlau 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 <bot@walbeck.it>
Co-committed-by: renovate-bot <bot@walbeck.it>
@toyobayashi toyobayashi mentioned this pull request Mar 29, 2023
Merged
@toyobayashi toyobayashi mentioned this pull request Apr 26, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhdawson mhdawson mhdawson approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@JckXia @mhdawson @nodejs-github-bot @jasnell @richardlau