src: simplify adding fast APIs to ExternalReferenceRegistry #58896

Renegade334 · 2025-06-29T22:24:12Z

The process for adding a fast API to the external registry is currently rather cumbersome, and involves:

adding the fast callback typedef to the long and ugly list of "allowed" CFunction signatures in node_external_reference.h
registering the fast callback
registering the v8::CFunctionInfo object obtained from the corresponding CFunction

This PR adds a new registry overload which simply takes a reference to the CFunction and registers both the callback pointer and the info pointer, without requiring any additional typedefs.

nodejs-github-bot · 2025-06-29T22:24:17Z

Review requested:

@nodejs/crypto
@nodejs/url

codecov · 2025-06-29T23:17:57Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.09%. Comparing base (4b4aaf9) to head (7cc286d).
⚠️ Report is 522 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #58896      +/-   ##
==========================================
- Coverage   90.11%   90.09%   -0.03%     
==========================================
  Files         640      640              
  Lines      188427   188396      -31     
  Branches    36965    36957       -8     
==========================================
- Hits       169804   169726      -78     
- Misses      11359    11388      +29     
- Partials     7264     7282      +18

Files with missing lines	Coverage Δ
src/crypto/crypto_timing.cc	`100.00% <100.00%> (ø)`
src/histogram.cc	`72.74% <100.00%> (-0.78%)`	⬇️
src/node_buffer.cc	`69.41% <100.00%> (+0.39%)`	⬆️
src/node_external_reference.h	`100.00% <100.00%> (ø)`
src/node_os.cc	`79.78% <100.00%> (-0.22%)`	⬇️
src/node_perf.cc	`86.38% <100.00%> (-0.06%)`	⬇️
src/node_process_methods.cc	`87.80% <100.00%> (-0.06%)`	⬇️
src/node_types.cc	`94.87% <100.00%> (-0.26%)`	⬇️
src/node_url.cc	`78.33% <100.00%> (-0.40%)`	⬇️
src/node_util.cc	`81.67% <100.00%> (-0.06%)`	⬇️
... and 3 more

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

src/node_external_reference.h

joyeecheung

I like how much simplification this brings! Though I'd agree with @legendecas that an overload of Register feels more natural.

src/node_external_reference.h

nodejs-github-bot · 2025-06-30T21:29:14Z

CI: https://ci.nodejs.org/job/node-test-pull-request/67757/

nodejs-github-bot · 2025-07-04T11:25:39Z

CI: https://ci.nodejs.org/job/node-test-pull-request/67836/

nodejs-github-bot · 2025-07-04T13:32:15Z

Landed in f64b0b3

PR-URL: #58896 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

PR-URL: nodejs#58896 Backport-PR-URL: nodejs#59054 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

PR-URL: nodejs#58896 Backport-PR-URL: https://github.com/nodejs/node/pull/XXXXX Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

PR-URL: nodejs#58896 Backport-PR-URL: nodejs#59065 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

PR-URL: nodejs#58896 Backport-PR-URL: nodejs#59065 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

nodejs/node#58896

* chore: bump node in DEPS to v22.18.0 * crypto: fix inclusion of OPENSSL_IS_BORINGSSL define nodejs/node#58845 * crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 nodejs/node#58960 * permission: propagate permission model flags on spawn nodejs/node#58853 * esm: syncify default path of ModuleLoader\.load nodejs/node#57419 * src: remove fast API for InternalModuleStat nodejs/node#58489 * src: simplify adding fast APIs to ExternalReferenceRegistry nodejs/node#58896 * chore: fixup patch indices * src: fix internalModuleStat v8 fast path nodejs/node#58054 * test: add tests to ensure that node.1 is kept in sync with cli.md nodejs/node#58878 * crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 nodejs/node#58942 --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <[email protected]>

chore: bump node to v22.18.0 (main) (#47937) * chore: bump node in DEPS to v22.18.0 * crypto: fix inclusion of OPENSSL_IS_BORINGSSL define nodejs/node#58845 * crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 nodejs/node#58960 * permission: propagate permission model flags on spawn nodejs/node#58853 * esm: syncify default path of ModuleLoader\.load nodejs/node#57419 * src: remove fast API for InternalModuleStat nodejs/node#58489 * src: simplify adding fast APIs to ExternalReferenceRegistry nodejs/node#58896 * chore: fixup patch indices * src: fix internalModuleStat v8 fast path nodejs/node#58054 * test: add tests to ensure that node.1 is kept in sync with cli.md nodejs/node#58878 * crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 nodejs/node#58942 --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <[email protected]>

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jun 29, 2025

legendecas reviewed Jun 30, 2025

View reviewed changes

src/node_external_reference.h Outdated Show resolved Hide resolved

joyeecheung approved these changes Jun 30, 2025

View reviewed changes

src/node_external_reference.h Outdated Show resolved Hide resolved

src: simplify adding fast APIs to ExternalReferenceRegistry

7cc286d

Renegade334 force-pushed the register-cfunction-reference branch from ce7bf56 to 7cc286d Compare June 30, 2025 21:16

joyeecheung approved these changes Jun 30, 2025

View reviewed changes

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 30, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 30, 2025

tniessen approved these changes Jul 1, 2025

View reviewed changes

legendecas approved these changes Jul 1, 2025

View reviewed changes

legendecas added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Jul 4, 2025

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 4, 2025

nodejs-github-bot merged commit f64b0b3 into nodejs:main Jul 4, 2025
64 checks passed

Renegade334 deleted the register-cfunction-reference branch July 4, 2025 13:38

github-actions bot mentioned this pull request Jul 8, 2025

2025-07-09, Version 24.4.0 (Current) #58993

Merged

Renegade334 mentioned this pull request Jul 14, 2025

[v22.x backport] src: backport fast API changes #59065

Merged

aduh95 added the backport-open-v22.x Indicate that the PR has an open backport label Jul 21, 2025

github-actions bot mentioned this pull request Jul 28, 2025

2025-07-29, Version 22.18.0 'Jod' (LTS) #59256

Merged

codebytere added a commit to electron/electron that referenced this pull request Aug 4, 2025

src: simplify adding fast APIs to ExternalReferenceRegistry

a0573f8

nodejs/node#58896

richardlau added backported-to-v22.x PRs backported to the v22.x-staging branch. and removed backport-open-v22.x Indicate that the PR has an open backport labels Sep 19, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

src: simplify adding fast APIs to ExternalReferenceRegistry #58896

src: simplify adding fast APIs to ExternalReferenceRegistry #58896

Renegade334 commented Jun 29, 2025 •

edited

Loading

Uh oh!

nodejs-github-bot commented Jun 29, 2025

Uh oh!

codecov bot commented Jun 29, 2025 •

edited

Loading

Uh oh!

Uh oh!

joyeecheung left a comment

Uh oh!

Uh oh!

nodejs-github-bot commented Jun 30, 2025

Uh oh!

nodejs-github-bot commented Jul 4, 2025

Uh oh!

Uh oh!

nodejs-github-bot commented Jul 4, 2025

Uh oh!

Uh oh!

Uh oh!

src: simplify adding fast APIs to ExternalReferenceRegistry #58896

src: simplify adding fast APIs to ExternalReferenceRegistry #58896

Conversation

Renegade334 commented Jun 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Jun 29, 2025

Uh oh!

codecov bot commented Jun 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

joyeecheung left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nodejs-github-bot commented Jun 30, 2025

Uh oh!

nodejs-github-bot commented Jul 4, 2025

Uh oh!

Uh oh!

nodejs-github-bot commented Jul 4, 2025

Uh oh!

Uh oh!

Renegade334 commented Jun 29, 2025 •

edited

Loading

codecov bot commented Jun 29, 2025 •

edited

Loading