-
Notifications
You must be signed in to change notification settings - Fork 4.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: make toBytes actually return the type it's typehint claims #29313
Conversation
Codecov Report
@@ Coverage Diff @@
## master #29313 +/- ##
=========================================
- Coverage 77.1% 76.5% -0.6%
=========================================
Files 55 54 -1
Lines 2934 3121 +187
Branches 408 468 +60
=========================================
+ Hits 2264 2390 +126
- Misses 529 567 +38
- Partials 141 164 +23 |
cee9820
to
032e3a2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! You're right that, while true that a Buffer
fully implements Uint8Array
from Typescript's perspective, folks have been bitten by that toString()
behaviour before.
Previously: a622198. |
76d4260
to
032e3a2
Compare
Pull request has been modified.
I've removed the updated to the lock files - they were not necessary for the fix itself, but were necessary for part of the CI to run which complained about outdated lock files |
Thanks for the fix! See you in a couple of weeks, when this breaks something we couldn't predict. |
Problem
toBytes claims to return a Uint8Array when in reality it returns a Buffer. This isn't a huge problem as in 99.9% of cases these two work just as well for the caller, but causes issues in some edge cases (e.g. toString() on a Uint8Array and on a Buffer over the same bytes returns different strings). It's also a code smell, due to making this method fully redundant to the toBuffer() method in addition to its typehint not returning exactly what it says it should.
Summary of Changes
Actually convert the buffer to a Uint8Array