nolanlawson/blob-util

Critiques and a feature suggestion

brodycj opened this issue · 2 comments

First, I would like to thank you for assembling and publishing this project.

There are a couple small things that could be improved:

  1. Certain functions return what the library user is looking for directly, and other functions return a promise that the library user has to deal with. The library user has to read through the documentation more carefully to get it right.
  2. A couple of functions loop through the contents in the Javascript, which can be slow for large objects. This should only be done in the case of old, broken browsers (assuming they will be fixed, someday).

For 1, I can think of several possibilities, in increasing order of favorability:

  • Break this into two projects (one for the functions that return promises and one for the functions that return the object directly) (not recommended)
  • Rename the functions to indicate which ones return a promise (a little better but still not recommended)
  • separate namespace or inner namespace for the functions that return a promise

For 2, I suggest the library checks the browser very carefully before actually looping through the contents in Javascript

A feature that could help some people is a pair of functions that provide the functionality of pouchdb/pouchdb#2900, to convert between a Blob and a binary string that can be stored in the existing Web SQL implementations (including my Cordova sqlite plugin). I may put this in a PR if I get a chance sometime.

Thanks for the suggestions! Responses:

Certain functions return what the library user is looking for directly, and other functions return a promise that the library user has to deal with.

Yeah, it's annoying, but I think the only solution would be to make every API return a promise for everything, which is kind of annoying. As of right now, you only need non-promises for createObjectURL, revokeObjectURL, and createBlob.

A couple of functions loop through the contents in the Javascript, which can be slow for large objects. This should only be done in the case of old, broken browsers (assuming they will be fixed, someday). Unless there's an API I missed?

On newer browsers I prefer the newer methods (e.g. readAsArrayBuffer() where it's available instead of readAsBinaryString()).

separate namespace or inner namespace for the functions that return a promise

I think sadly the only solution to this problem will come with ES7 when we have async functions. Then I can just mark the promise-returning functions as async and it will be crystal clear because your IDE will show an error if you try to use an async function as a function or vice-versa.

In general I would like to increase the modularity of this project by breaking everything up into separate files, so that you can e.g. require('blob-util/blobToBinaryString') and only use the functions that you need. I would happily accept a PR to change it that way! :)

Agreed with your idea to increase the modularity, I don't expect to have time to work on this one. You may want to open a new issue for this with a "help-wanted" label.