Skip to content

embedder: return empty pointer without builtin snapshot - #64845

Open
Archkon wants to merge 1 commit into
nodejs:mainfrom
Archkon:embedder
Open

embedder: return empty pointer without builtin snapshot#64845
Archkon wants to merge 1 commit into
nodejs:mainfrom
Archkon:embedder

Conversation

@Archkon

@Archkon Archkon commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Avoid creating an EmbedderSnapshotData wrapper with null implementation when Node.js is built without a builtin snapshot. This ensures callers can reliably detect the unavailable snapshot through the public API.

@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 Jul 30, 2026
Avoid creating an EmbedderSnapshotData wrapper with null implementation
when Node.js is built without a builtin snapshot. This ensures callers
can reliably detect the unavailable snapshot through the public API.

Signed-off-by: Archkon <180910180+Archkon@users.noreply.github.com>
@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.20%. Comparing base (0012a77) to head (dffb2c8).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
src/api/embed_helpers.cc 0.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64845      +/-   ##
==========================================
+ Coverage   90.14%   90.20%   +0.05%     
==========================================
  Files         746      746              
  Lines      242771   242931     +160     
  Branches    45741    45803      +62     
==========================================
+ Hits       218850   219134     +284     
+ Misses      15426    15331      -95     
+ Partials     8495     8466      -29     
Files with missing lines Coverage Δ
src/node.h 92.30% <ø> (ø)
src/api/embed_helpers.cc 73.33% <0.00%> (-1.79%) ⬇️

... and 38 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.

@saitejabandaru-in saitejabandaru-in left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM! Returning an empty Pointer explicitly when snapshot_data is nullptr safely avoids wrapping a null implementation, aligning with the expected behavior of EmbedderSnapshotData::BuiltinSnapshotData when the snapshot is missing.

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

Labels

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.

3 participants