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

feat(napi): property getter and setter with closure #1526

Merged

Brooooooklyn merged 7 commits into napi-rs:main from victorteokw:property_closure

Mar 21, 2023

Contributor

victorteokw commented Mar 19, 2023 •

edited

This PR tries to implement issue #1378

victorteokw mentioned this pull request

Pass closure for Property setter/getter #1378

Closed

victorteokw changed the title ~~getter with closure with segment fault~~ property getter and setter with closure

Brooooooklyn reviewed

View reviewed changes

crates/napi/src/js_values/object_property.rs Outdated Show resolved Hide resolved

Brooooooklyn reviewed

View reviewed changes

crates/napi/src/js_values/object_property.rs Outdated Show resolved Hide resolved

Contributor Author

victorteokw commented Mar 20, 2023

Hi @Brooooooklyn, Thanks for the review! 👏

I'll update the code. By the way, we may need someway to detect these and register the cleanup from define_properties. It's all fine if a property with setter and getter closure is defined for a class prototype. If somebody defines this on plain objects, it may leak a little heap memory.

victorteokw and others added 4 commits

March 20, 2023 21:50


          getter with closure with segment fault

f5437fc


          fix getter closure pointer

33d5339


          add setter

ecf9958


          Cleanup API

96e0a6f

Brooooooklyn force-pushed the property_closure branch from 205a4d5 to 96e0a6f Compare

March 20, 2023 16:20

Sponsor Member

Brooooooklyn commented Mar 20, 2023

@victorteokw I've cleaned up your implementation and added some tests 😁 I'm going to merge this after all checks pass

Brooooooklyn added 3 commits

March 21, 2023 00:57


          Add test for create_function_from_closure

71e234c


          Fix compile error

a4a6b75


          Fix flaky test title

7a2804e

Contributor Author

victorteokw commented Mar 21, 2023

😻 Thanks! @Brooooooklyn

Brooooooklyn changed the title ~~property getter and setter with closure~~ feat(napi): property getter and setter with closure

Brooooooklyn approved these changes

View reviewed changes

Brooooooklyn merged commit aee742f into napi-rs:main

32 of 33 checks passed

MarkusJx mentioned this pull request

fix(napi): access violation in property getter/setter closure #1552

Merged

kodiakhq bot pushed a commit to X-oss-byte/Canary-nextjs that referenced this pull request


          Update dependency @napi-rs/cli to v2.16.3 (#114)

9a7f849

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [@napi-rs/cli](https://togithub.com/napi-rs/napi-rs) | [`2.13.3` -> `2.16.3`](https://renovatebot.com/diffs/npm/@napi-rs%2fcli/2.13.3/2.16.3) | [![age](https://developer.mend.io/api/mc/badges/age/npm/@napi-rs%2fcli/2.16.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@napi-rs%2fcli/2.16.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@napi-rs%2fcli/2.13.3/2.16.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@napi-rs%2fcli/2.13.3/2.16.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>napi-rs/napi-rs (@&#8203;napi-rs/cli)</summary>

### [`v2.16.3`](https://togithub.com/napi-rs/napi-rs/releases/tag/%40napi-rs/cli%402.16.3)

[Compare Source](https://togithub.com/napi-rs/napi-rs/compare/@napi-rs/cli@2.16.2...@napi-rs/cli@2.16.3)

#### What's Changed

-   chore: upgrade npm dependencies by [@&#8203;Brooooooklyn](https://togithub.com/Brooooooklyn) in [napi-rs/napi-rs#1658
-   fix(cli): zig 0.11 link args by [@&#8203;Brooooooklyn](https://togithub.com/Brooooooklyn) in [napi-rs/napi-rs#1698

**Full Changelog**: https://github.com/napi-rs/napi-rs/compare/[@&#8203;napi-rs/cli](https://togithub.com/napi-rs/cli)[@&#8203;2](https://togithub.com/2).16.2...[@&#8203;napi-rs/cli](https://togithub.com/napi-rs/cli)[@&#8203;2](https://togithub.com/2).16.3

### [`v2.16.2`](https://togithub.com/napi-rs/napi-rs/releases/tag/%40napi-rs/cli%402.16.2)

[Compare Source](https://togithub.com/napi-rs/napi-rs/compare/@napi-rs/cli@2.16.1...@napi-rs/cli@2.16.2)

#### What's Changed

-   fix(cli): ts constEnum option not working when set in package.json by [@&#8203;pengx17](https://togithub.com/pengx17) in [napi-rs/napi-rs#1647

#### New Contributors

-   [@&#8203;pengx17](https://togithub.com/pengx17) made their first contribution in [napi-rs/napi-rs#1647

**Full Changelog**: https://github.com/napi-rs/napi-rs/compare/[@&#8203;napi-rs/cli](https://togithub.com/napi-rs/cli)[@&#8203;2](https://togithub.com/2).16.1...[@&#8203;napi-rs/cli](https://togithub.com/napi-rs/cli)[@&#8203;2](https://togithub.com/2).16.2

### [`v2.16.1`](https://togithub.com/napi-rs/napi-rs/compare/@napi-rs/cli@2.16.0...@napi-rs/cli@2.16.1)

[Compare Source](https://togithub.com/napi-rs/napi-rs/compare/@napi-rs/cli@2.16.0...@napi-rs/cli@2.16.1)

### [`v2.16.0`](https://togithub.com/napi-rs/napi-rs/releases/tag/%40napi-rs/cli%402.16.0)

[Compare Source](https://togithub.com/napi-rs/napi-rs/compare/@napi-rs/cli@2.15.2...@napi-rs/cli@2.16.0)

#### What's Changed

-   feat(cli): support --profile flag by [@&#8203;Brooooooklyn](https://togithub.com/Brooooooklyn)  napi-rs/napi-rs@dfbe464

**Full Changelog**: https://github.com/napi-rs/napi-rs/compare/[@&#8203;napi-rs/cli](https://togithub.com/napi-rs/cli)[@&#8203;2](https://togithub.com/2).15.2...[@&#8203;napi-rs/cli](https://togithub.com/napi-rs/cli)[@&#8203;2](https://togithub.com/2).16.0

### [`v2.15.2`](https://togithub.com/napi-rs/napi-rs/releases/tag/%40napi-rs/cli%402.15.2)

[Compare Source](https://togithub.com/napi-rs/napi-rs/compare/@napi-rs/cli@2.15.1...@napi-rs/cli@2.15.2)

#### What's Changed

-   fix(cli,napi-derive): backward compatible with older cli with [#&#8203;1531](https://togithub.com/napi-rs/napi-rs/issues/1531) by [@&#8203;Brooooooklyn](https://togithub.com/Brooooooklyn) in [napi-rs/napi-rs#1536

**Full Changelog**: https://github.com/napi-rs/napi-rs/compare/napi-derive@2.12.1...[@&#8203;napi-rs/cli](https://togithub.com/napi-rs/cli)[@&#8203;2](https://togithub.com/2).15.2

### [`v2.15.1`](https://togithub.com/napi-rs/napi-rs/releases/tag/%40napi-rs/cli%402.15.1)

[Compare Source](https://togithub.com/napi-rs/napi-rs/compare/@napi-rs/cli@2.15.0...@napi-rs/cli@2.15.1)

#### What's Changed

-   fix(cli,napi-derive): re-export types from shared crate by [@&#8203;Brooooooklyn](https://togithub.com/Brooooooklyn) in [napi-rs/napi-rs#1531
-   fix(cli): dts pipe by [@&#8203;Brooooooklyn](https://togithub.com/Brooooooklyn) in [napi-rs/napi-rs#1532

**Full Changelog**: https://github.com/napi-rs/napi-rs/compare/napi-derive@2.12.0...[@&#8203;napi-rs/cli](https://togithub.com/napi-rs/cli)[@&#8203;2](https://togithub.com/2).15.1

### [`v2.15.0`](https://togithub.com/napi-rs/napi-rs/releases/tag/%40napi-rs/cli%402.15.0)

[Compare Source](https://togithub.com/napi-rs/napi-rs/compare/@napi-rs/cli@2.14.8...@napi-rs/cli@2.15.0)

#### What's Changed

-   fix(cli): export non const enums when generating typedefs by [@&#8203;nebarf](https://togithub.com/nebarf) in [napi-rs/napi-rs#1527
-   fix(napi): ThreadsafeFunctionHandle never being dropped by [@&#8203;Brooooooklyn](https://togithub.com/Brooooooklyn) in [napi-rs/napi-rs#1530
-   feat: export registers in wasm32 target by [@&#8203;Brooooooklyn](https://togithub.com/Brooooooklyn) in [napi-rs/napi-rs#1529

#### New Contributors

-   [@&#8203;meowtec](https://togithub.com/meowtec) made their first contribution in [napi-rs/napi-rs#1525
-   [@&#8203;nebarf](https://togithub.com/nebarf) made their first contribution in [napi-rs/napi-rs#1527
-   [@&#8203;victorteokw](https://togithub.com/victorteokw) made their first contribution in [napi-rs/napi-rs#1526

**Full Changelog**: https://github.com/napi-rs/napi-rs/compare/napi@2.11.4...[@&#8203;napi-rs/cli](https://togithub.com/napi-rs/cli)[@&#8203;2](https://togithub.com/2).15.0

### [`v2.14.8`](https://togithub.com/napi-rs/napi-rs/releases/tag/%40napi-rs/cli%402.14.8)

[Compare Source](https://togithub.com/napi-rs/napi-rs/compare/@napi-rs/cli@2.14.7...@napi-rs/cli@2.14.8)

##### Bug Fixes

-   **cli:** always generate typedef file even native code never changes ([#&#8203;1489](https://togithub.com/napi-rs/napi-rs/issues/1489)) ([7281f53](https://togithub.com/napi-rs/napi-rs/commit/7281f533bd73d6c6255244c9f1556a0e39c47738))

### [`v2.14.7`](https://togithub.com/napi-rs/napi-rs/releases/tag/%40napi-rs/cli%402.14.7)

[Compare Source](https://togithub.com/napi-rs/napi-rs/compare/@napi-rs/cli@2.14.6...@napi-rs/cli@2.14.7)

##### Bug Fixes

-   **cli:** JS bindgen file now respects destDir ([#&#8203;1481](https://togithub.com/napi-rs/napi-rs/issues/1481)) ([cb529d2](https://togithub.com/napi-rs/napi-rs/commit/cb529d21cf92dfaa279bfe10dde5ad87441b47ba))

### [`v2.14.6`](https://togithub.com/napi-rs/napi-rs/releases/tag/%40napi-rs/cli%402.14.6)

[Compare Source](https://togithub.com/napi-rs/napi-rs/compare/@napi-rs/cli@2.14.5...@napi-rs/cli@2.14.6)

**Note:** Version bump only for package [@&#8203;napi-rs/cli](https://togithub.com/napi-rs/cli)

### [`v2.14.5`](https://togithub.com/napi-rs/napi-rs/releases/tag/%40napi-rs/cli%402.14.5)

[Compare Source](https://togithub.com/napi-rs/napi-rs/compare/@napi-rs/cli@2.14.4...@napi-rs/cli@2.14.5)

##### Bug Fixes

-   **cli:** upload GitHub assets issue ([a302c9c](https://togithub.com/napi-rs/napi-rs/commit/a302c9cb18710d8d71045d11780f09d4eaf1ecde))

### [`v2.14.4`](https://togithub.com/napi-rs/napi-rs/releases/tag/%40napi-rs/cli%402.14.4)

[Compare Source](https://togithub.com/napi-rs/napi-rs/compare/@napi-rs/cli@2.14.3...@napi-rs/cli@2.14.4)

##### Bug Fixes

-   **napi:** build error with zig@0.10.1 ([2f52793](https://togithub.com/napi-rs/napi-rs/commit/2f527938b27074116a89b9248e218a2ed073be34))

### [`v2.14.3`](https://togithub.com/napi-rs/napi-rs/releases/tag/%40napi-rs/cli%402.14.3)

[Compare Source](https://togithub.com/napi-rs/napi-rs/compare/@napi-rs/cli@2.14.2...@napi-rs/cli@2.14.3)

#### What's Changed

-   Fix broken zig behaviors by [@&#8203;controversial](https://togithub.com/controversial) in [napi-rs/napi-rs#1432

#### New Contributors

-   [@&#8203;controversial](https://togithub.com/controversial) made their first contribution in [napi-rs/napi-rs#1432

**Full Changelog**: https://github.com/napi-rs/napi-rs/compare/napi@2.10.6...[@&#8203;napi-rs/cli](https://togithub.com/napi-rs/cli)[@&#8203;2](https://togithub.com/2).14.3

### [`v2.14.2`](https://togithub.com/napi-rs/napi-rs/releases/tag/%40napi-rs/cli%402.14.2)

[Compare Source](https://togithub.com/napi-rs/napi-rs/compare/@napi-rs/cli@2.14.1...@napi-rs/cli@2.14.2)

#### What's Changed

-   Fix CARGO env var in `napi build` by [@&#8203;overlookmotel](https://togithub.com/overlookmotel) in [napi-rs/napi-rs#1426

**Full Changelog**: https://github.com/napi-rs/napi-rs/compare/napi@2.10.5...[@&#8203;napi-rs/cli](https://togithub.com/napi-rs/cli)[@&#8203;2](https://togithub.com/2).14.2

### [`v2.14.1`](https://togithub.com/napi-rs/napi-rs/releases/tag/%40napi-rs/cli%402.14.1)

[Compare Source](https://togithub.com/napi-rs/napi-rs/compare/@napi-rs/cli@2.14.0...@napi-rs/cli@2.14.1)

#### What's Changed

-   \[Fix] Quote toml path by [@&#8203;TheBrenny](https://togithub.com/TheBrenny) in [napi-rs/napi-rs#1410
-   chore(cli): update CI template by [@&#8203;Brooooooklyn](https://togithub.com/Brooooooklyn) in [napi-rs/napi-rs#1416

#### New Contributors

-   [@&#8203;TheBrenny](https://togithub.com/TheBrenny) made their first contribution in [napi-rs/napi-rs#1410

**Full Changelog**: https://github.com/napi-rs/napi-rs/compare/napi@2.10.4...[@&#8203;napi-rs/cli](https://togithub.com/napi-rs/cli)[@&#8203;2](https://togithub.com/2).14.1

### [`v2.14.0`](https://togithub.com/napi-rs/napi-rs/releases/tag/%40napi-rs/cli%402.14.0)

[Compare Source](https://togithub.com/napi-rs/napi-rs/compare/@napi-rs/cli@2.13.3...@napi-rs/cli@2.14.0)

#### What's Changed

-   feat(cli): `Darwin` universal architecture by [@&#8203;skirsdeda](https://togithub.com/skirsdeda) in [napi-rs/napi-rs#1397
-   feat(cli): add --zig-link-only option by [@&#8203;Brooooooklyn](https://togithub.com/Brooooooklyn) in [napi-rs/napi-rs#1400

#### New Contributors

-   [@&#8203;skirsdeda](https://togithub.com/skirsdeda) made their first contribution in [napi-rs/napi-rs#1397

**Full Changelog**: https://github.com/napi-rs/napi-rs/compare/napi@2.10.3...[@&#8203;napi-rs/cli](https://togithub.com/napi-rs/cli)[@&#8203;2](https://togithub.com/2).14.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - 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.

---

 - [ ] If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/X-oss-byte/Canary-nextjs).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment