FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix Bun compatibility and N-API reference teardown by yumin-chen · Pull Request #7 · Chen-Software/nodegit · GitHub

Repository navigation

Fix Bun compatibility and N-API reference teardown - #7

Open
yumin-chen wants to merge 7 commits into
napifrom
fix/bun-compatibility-883390392492057550
Open

yumin-chen wants to merge 7 commits into
napifrom
fix/bun-compatibility-883390392492057550

Conversation

Copy link
Copy Markdown

This change fixes compatibility issues when running NodeGit under Bun and addresses review queries:

  1. Bun OpenSSL Symbol Resolution: On Linux non-Electron builds, -lcrypto and -lssl are linked in binding.gyp so that runtimes like Bun (which do not export OpenSSL/BoringSSL symbols globally) can load nodegit.node via dlopen without undefined symbol: EVP_des_ede3_cbc errors.
  2. N-API Reference Cleanup: Added owner.SuppressDestruct() to Napi::Persistent references in NodeGitWrapper to prevent V8 HandleScope crashes during GC and environment teardown.
  3. Reference Callback Safety: Added null checks if (i.second) prior to executing referenceCallbacks and unreferenceCallbacks to avoid std::bad_function_call exceptions.
  4. Test Fixes: Replaced invalid assert.false and assert.gt methods in test/tests/clone.js with standard Node.js assert.strictEqual and assert.ok.
  5. N-API Header Clarification: Clarified that "napi.h" is the official C++ wrapper (node-addon-api) over low-level C <node/node_api.h>, making "napi.h" the standard C++ interface for Node-API.

PR created automatically by Jules for task 883390392492057550 started by @yumin-chen

yumin-chen and others added 6 commits September 18, 2026 11:55
Require node >= 22 and use NAPI_VERSION=10
- Link -lcrypto and -lssl on Linux non-Electron builds in binding.gyp so host engines like Bun can dlopen nodegit.node
- Call owner.SuppressDestruct() in NodeGitWrapper to prevent HandleScope errors during environment teardown
- Check callback validity in reference/unreference callbacks to avoid bad_function_call exceptions
- Fix invalid assert.false and assert.gt calls in test/tests/clone.js

Co-authored-by: yumin-chen <10954839+yumin-chen@users.noreply.github.com>

Copy link
Copy Markdown

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

- Link -lcrypto and -lssl on Linux non-Electron builds in binding.gyp so host engines like Bun can dlopen nodegit.node
- Wrap owner in std::unique_ptr<Napi::ObjectReference> in NodeGitWrapper and call SuppressDestruct() to prevent HandleScope errors during environment teardown
- Call SuppressDestruct() and delete underlying reference in TrackerWrap destructor
- Check callback validity in reference/unreference callbacks to avoid bad_function_call exceptions
- Fix invalid assert.false and assert.gt calls in test/tests/clone.js

Co-authored-by: yumin-chen <10954839+yumin-chen@users.noreply.github.com>
yumin-chen force-pushed the napi branch 5 times, most recently from f99272f to ba0e77d Compare September 21, 2026 14:26
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant


Back | FazBrowse Home | New Git URL