buffer: add Buffer.fromHex() and Buffer.fromBase64()

[LiviaMedeiros]

These methods are already present on Uint8Array, so without defining them on Buffer they return instances of Uint8Array. This introduces wrapper to return instances of Buffer instead.

Fixes: #61146

[codecov]

Codecov Report

❌ Patch coverage is 74.35897% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.52%. Comparing base (55600e6) to head (d2d0ae2).
⚠️ Report is 63 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/buffer.js	74.35%	10 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##             main   #61157    +/-   ##
========================================
  Coverage   88.52%   88.52%            
========================================
  Files         703      704     +1     
  Lines      208589   208798   +209     
  Branches    40226    40275    +49     
========================================
+ Hits       184650   184841   +191     
- Misses      15954    15955     +1     
- Partials     7985     8002    +17     

Files with missing lines	Coverage Δ
lib/buffer.js	99.29% <74.35%> (-0.71%)	⬇️

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

[avivkeller]

Remember to update the docs

[avivkeller]

Other than this is missing a doc entry, LGTM

[LiviaMedeiros]

Agreed, added to the docs.
Initially i intentionally omitted it because these are basically inherited from Uint8Array (we don't document Buffer.prototype.at() whatsoever), but it's worth adding to indicate that these are similar-but-different from Buffer.from(string, encoding).

[avivkeller]

Docs + impl lgtm

[nodejs-github-bot]

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

[aduh95]

Should we define those only if --no-js-base-64 as it is the case today?

$ node --no-js-base-64 -p 'Buffer.fromHex'                         
undefined
$ node -p 'Buffer.fromHex'    
[Function: fromHex]

[aduh95]

I don't think lazy loading makes sense here, I don't see what's the upside. Why not put it under an if statement instead?

$ ./node -p '"fromHex" in Buffer'
true
$ ./node --no-js-base-64 -p '"fromHex" in Buffer'
false

[LiviaMedeiros]

@aduh95 IIUC buffer.js is a part of early bootstrap, which is why getOptionValue('--js-base-64') is not available yet, and Uint8Array.{fromHex,fromBase64} are still undefined even when the feature is enabled. Neither

'fromHex' in Uint8Array
Uint8Array.fromHex !== undefined
typeof Uint8Array.fromHex !== 'undefined'
const { Uint8Array: { fromHex } } = primordials
const { globalThis: { Uint8Array: { fromHex } } } = primordials
const { Uint8Array: { fromHex } } = globalThis

gives anything meaningful.
If I'm missing something and it is possible to check synchronously, please let me know, I'd be happy to replace it with if.

[targos]

Why do we care about --js-base-64? We only explicitly support a small subset of V8 flags and I don't see a compelling reason in this case.

[LiviaMedeiros]

const { fromHex: Uint8ArrayFromHex, fromBase64: Uint8ArrayFromBase64 } = Uint8Array;

I would love to do this if both variables didn't end up as undefined. It works in userspace, but not in lib/buffer.js. :(

[Renegade334]

I still don't really see what usage case this is intended for, given that Buffer already has long-established patterns for interacting with base64.

However, even if this is deemed worthwhile, I can't see that it's worthwhile enough to be bending over backwards to hack around the issues of shadowing a flagged feature. I feel like the best approach is to leave this alone, if at least until the feature is unflagged.

Users can easily replicate this functionality themselves by changing the prototype of their returned Uint8Array, if their usage case is truly not amenable to Buffer.from().

[LiviaMedeiros]

The reasoning for this comes from #61146. It's not really a new feature to be used with Buffer, but more of a bugfix to make already existing function return value of correct type. Buffer.isBuffer(Buffer.fromHex('aa')) === false doesn't make sense.

Uint8Array.fromHex and Uint8Array.fromBase64 are enabled by default right now, so I think it's better to fix it right away (even though the lazy loading workaround indeed looks annoying), and replace with prettier solution once it's available. My personal preference would be Buffer.fromHex &&= ..., making future follow-up a two-line patch.

Agreed that this function is awkward in terms of actual usage (users who need Buffer would usually stick to Buffer.from(..., 'hex'), users who need fromHex() would usually stick to Uint8Arrays), but IMHO it's a valid reason to not worry about optimizing its performance rather than reason to return wrong type.

[aduh95]

t really see what usage case this is intended for, given that Buffer already has long-established patterns for interacting with base64.

I can't reproduce, they are defined for me (unless I pass --no-js-base-64)