fix: Replace SafeBuffer with Buffer, support BASE64 in the browser #74
Conversation
index.js
Outdated
| function decodeBase64(base64) { | ||
| return new Buffer(base64, 'base64').toString(); | ||
| } : |
There was a problem hiding this comment.
| function decodeBase64(base64) { | |
| return new Buffer(base64, 'base64').toString(); | |
| } : | |
| function decodeBase64(base64) { | |
| if (typeof value === 'number') { | |
| throw new TypeError('The value to decode must not be of type number.) | |
| } | |
| return new Buffer(base64, 'base64').toString(); | |
| } : |
safer-buffer checks to make sure that the value isn't a number (I believe that is because new Buffer allocates bytes of length value if the value is a number).
index.js
Outdated
| var decodeBase64 = typeof Buffer !== 'undefined' ? Buffer.from ? | ||
| function decodeBase64(base64) { | ||
| return Buffer.from(base64, 'base64').toString(); | ||
| } : |
There was a problem hiding this comment.
Can we make these if statements instead of ternary?
index.js
Outdated
| Converter.prototype.toBase64 = typeof Buffer !== 'undefined' ? Buffer.from ? | ||
| function () { |
There was a problem hiding this comment.
Can these be if statements too?
There was a problem hiding this comment.
Agreed + this function should be declared elsewhere and named and then just referenced here, otherwise it is hard to debug since it'll be anonymous.
Additionally probably should do Buffer.from.bind(Buffer just in case in the future that function will refer to Buffer as this.
This is a part of changes to get this module usable in the web browser and more lightweight at the same time. * Remove the safe-buffer dependency. Use the Bufer constructor and Buffer.from depending on what is available. * Use atob/btoa with a workaround for UTF-8 in the browser.
phated
force-pushed
the
remove-safe-buffer
branch
from
c2919ce to
cb8f059
Compare
|
@thlorenz I rebased on top of the GitHub Actions and updated the code based on what we discussed above. Can you take another look? |
|
Some of the CI is failing on this one now .. not sure if it's a fluke, but at any rate we could probably take anything |
Windows + node <4 is really flakey. I think we should remove them when we bump the 2.0 major (which I'll begin prepping after we release this). Sound good? |
This is a part of changes to get this module usable in the web browser
and more lightweight at the same time.
Buffer.from depending on what is available.
Fixes #70 without dropping the compatibility with Node.js 0.10.