-
-
Notifications
You must be signed in to change notification settings - Fork 12
convert anything to Buffer with promise #21
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,21 +1,3 @@ | ||
/* global Blob, FileReader */ | ||
/* global Response, Buffer */ | ||
|
||
module.exports = function blobToBuffer (blob, cb) { | ||
if (typeof Blob === 'undefined' || !(blob instanceof Blob)) { | ||
throw new Error('first argument must be a Blob') | ||
} | ||
if (typeof cb !== 'function') { | ||
throw new Error('second argument must be a function') | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we keep this check in, but reverse the condition? It would be nice to help folks who are upgrading from 1.x and might leave the callback in by accident? It's similar to how we left the check in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mm, don't follow... 😕 maybe close this PR just and let you do as you want? |
||
|
||
const reader = new FileReader() | ||
|
||
function onLoadEnd (e) { | ||
reader.removeEventListener('loadend', onLoadEnd, false) | ||
if (e.error) cb(e.error) | ||
else cb(null, Buffer.from(reader.result)) | ||
} | ||
|
||
reader.addEventListener('loadend', onLoadEnd, false) | ||
reader.readAsArrayBuffer(blob) | ||
} | ||
module.exports = anything => new Response(anything).arrayBuffer().then(Buffer.from) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer this function to be named so it shows up with a name in stack traces. Also, I know that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
{ | ||
"name": "blob-to-buffer", | ||
"description": "Convert a Blob to a Buffer", | ||
"version": "1.2.8", | ||
"version": "2.0.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please don't bump the version in a PR. This should happen when the new version is actually published. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure |
||
"author": { | ||
"name": "Feross Aboukhadijeh", | ||
"email": "[email protected]", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,22 +3,11 @@ | |
const toBuffer = require('../') | ||
const test = require('tape') | ||
|
||
const blob = new Blob([new Uint8Array([1, 2, 3])], { type: 'application/octet-binary' }) | ||
const blob = new Blob([new Uint8Array([1, 2, 3])]) | ||
|
||
test('Basic tests', function (t) { | ||
toBuffer(blob, function (err, buffer) { | ||
if (err) throw err | ||
toBuffer(blob).then(buffer => { | ||
t.deepEqual(buffer, Buffer.from([1, 2, 3])) | ||
t.end() | ||
}) | ||
}) | ||
|
||
test('Callback error on invalid arguments', function (t) { | ||
t.throws(function () { | ||
toBuffer({ blah: 1 }, function () {}) | ||
}) | ||
t.throws(function () { | ||
toBuffer(blob) | ||
}) | ||
t.end() | ||
}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was this test removed? What happens when an invalid argument like an object is passed in? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It would be silly to just restrict it to only Blobs when you can accept many more types of stuff and let it be more loose and be used for more than just Blobs. anything not understood by the Response constructor will be casted to a string using the so what happens when you do
So it's a bit hard to get a error when Response can accept literally anything So it's no longer just blob-to-buffer, it's now anything to buffer |
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.
Can you switch this example to use
await
?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.
sure, i only used
.then()
as it was more cross platform compatible and many ppl just copy/paste code without using babel