Skip to content

Comments

fix(NODE-7414): add copy method to ByteUtils#867

Merged
baileympearson merged 7 commits intomainfrom
add-copy
Jan 29, 2026
Merged

fix(NODE-7414): add copy method to ByteUtils#867
baileympearson merged 7 commits intomainfrom
add-copy

Conversation

@PavelSafronov
Copy link
Contributor

@PavelSafronov PavelSafronov commented Jan 26, 2026

Description

Summary of Changes

Add copy method to ByteUtils, with web and Node implementations.

What is the motivation for this change?

Add a common copy method which will be used by Node Driver

Release Highlight

Buffer.copy() now present in ByteUtils

ByteUtils now contains a copy() method, we behaves identically to Nodejs' Buffer.copy() method.

Double check the following

  • Lint is passing (npm run check:lint)
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@PavelSafronov PavelSafronov marked this pull request as ready for review January 26, 2026 22:19
@PavelSafronov PavelSafronov requested a review from a team as a code owner January 26, 2026 22:19
@baileympearson baileympearson added the Team Review Needs review from team label Jan 28, 2026
@baileympearson baileympearson self-assigned this Jan 28, 2026
@baileympearson baileympearson requested review from a team, JamesKovacs, RaschidJFR and tadjik1 and removed request for a team January 28, 2026 16:43
Copy link
Contributor

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

Does this need a separate implementation for Node.js? Always using the web one would result in consistent validation and otherwise no major downsides, right?

@dariakp dariakp requested review from a team and removed request for JamesKovacs, RaschidJFR and tadjik1 January 28, 2026 17:09
@PavelSafronov
Copy link
Contributor Author

Does this need a separate implementation for Node.js? Always using the web one would result in consistent validation and otherwise no major downsides, right?

We can use the web implementation in both cases, but I don't think we should. Other operations in ‎src/utils/node_byte_utils.ts‎ use Buffer.* methods, like the below implementation of concat, and I think we want to keep this approach, in part so we can get the benefit of various Buffer.* optimizations that may be present in Node.

concat(list: Uint8Array[]): NodeJsBuffer {
    return Buffer.concat(list);
  }

Copy link
Member

@tadjik1 tadjik1 left a comment

Choose a reason for hiding this comment

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

Great implementation, decent test coverage, well done @PavelSafronov!

@baileympearson baileympearson merged commit ffa77c6 into main Jan 29, 2026
9 checks passed
@baileympearson baileympearson deleted the add-copy branch January 29, 2026 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Team Review Needs review from team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants