Skip to content

src: fix cjs_lexer external reference registration#61718

Open
joyeecheung wants to merge 1 commit intonodejs:mainfrom
joyeecheung:cjs-ref
Open

src: fix cjs_lexer external reference registration#61718
joyeecheung wants to merge 1 commit intonodejs:mainfrom
joyeecheung:cjs-ref

Conversation

@joyeecheung
Copy link
Member

It needs to be added to the list to actually get registered.

It needs to be added to the list to actually get registered.
@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 Feb 7, 2026
@addaleax addaleax added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Feb 7, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 7, 2026
@nodejs-github-bot
Copy link
Collaborator

@codecov
Copy link

codecov bot commented Feb 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.73%. Comparing base (8e41b8d) to head (007b4aa).
⚠️ Report is 53 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #61718      +/-   ##
==========================================
- Coverage   89.74%   89.73%   -0.01%     
==========================================
  Files         674      675       +1     
  Lines      204348   204502     +154     
  Branches    39271    39305      +34     
==========================================
+ Hits       183396   183519     +123     
- Misses      13262    13273      +11     
- Partials     7690     7710      +20     
Files with missing lines Coverage Δ
src/node_external_reference.h 100.00% <ø> (ø)

... and 59 files with indirect coverage changes

🚀 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.

Copy link
Member

@anonrig anonrig left a comment

Choose a reason for hiding this comment

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

For educational purposes: How did you encounter this issue, and what problem did you see?

@joyeecheung
Copy link
Member Author

joyeecheung commented Feb 7, 2026

I was trying to create a snapshot with the ESM loader. When it's not properly registered adding it to the snapshot would break (see src/README.md's section about external references on what it looks like, or with my PR in #61719 it will suggest the binding is not in there).

@anonrig
Copy link
Member

anonrig commented Feb 7, 2026

I was trying to create a snapshot with the ESM loader. When it's not properly registered adding it to the snapshot would break (see src/README.md's section about external references on what it looks like, or with my PR in #61719 it will suggest the binding is not in there).

Thanks!

@nodejs-github-bot
Copy link
Collaborator

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

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants