Skip to content

Conversation

legendecas
Copy link
Member

@legendecas legendecas commented Jun 19, 2025

Fixes: #1661

  • Add docs.

@codecov-commenter
Copy link

codecov-commenter commented Jun 19, 2025

Codecov Report

❌ Patch coverage is 40.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.79%. Comparing base (a6213bb) to head (d07f9d7).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
napi-inl.h 40.00% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1670   +/-   ##
=======================================
  Coverage   65.79%   65.79%           
=======================================
  Files           3        3           
  Lines        2146     2149    +3     
  Branches      715      716    +1     
=======================================
+ Hits         1412     1414    +2     
  Misses        162      162           
- Partials      572      573    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@legendecas legendecas force-pushed the constructor-traits branch from 6c7c39f to d07f9d7 Compare June 19, 2025 16:38
@legendecas legendecas changed the title feat: add ConstructorTrait to allow constructor return overridding feat: add ConstructorTraitTag to allow constructor return overridding Jun 19, 2025
@legendecas
Copy link
Member Author

This does not work for InstanceMethod because there is a branch check enabled at https://github.com/nodejs/node/blob/main/src/js_native_api_v8.cc#L1028.

@greggman
Copy link

Thank you for working on this!

It's not clear from the comments if this is working yet. It sounds like maybe not?

I would suggest adding a few more tests

  • Make a C++ class that inherits from Event, then pass it to a native event target

    const myEvent = new CPPClassThatInheritsFromEvent('nameOfEvent');
    const target = new EventTarget();
    const e = await new Promise(resolve => {
      target.addEventListener('nameOfEvent', resolve, { once: true });
      target.dispatchEvent(myEvent);
    });
    assert(e === myEvent);
  • Make a C++ class that inherits from EventTarget. Maybe sure you can use EventTarget's functionality

    const target = new CPPClassThatInheritsFromEventTarget();
    const event = new Event('foo');
    const e = await new Promise(resolve => {
      target.addEventListener('foo', resolve, { once: true });
      target.dispatchEvent(event);
    });
    assert(e === event);

@legendecas legendecas moved this from Need Triage to In Progress in Node-API Team Project Jul 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

Wrapping Event in a C++ class so that every where you pass an event it works.
3 participants