Skip to content
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

Array Buffers #34

Closed
calvinmetcalf opened this issue Jul 29, 2014 · 13 comments
Closed

Array Buffers #34

calvinmetcalf opened this issue Jul 29, 2014 · 13 comments
Labels
upgrade guide Improvements or additions to upgrade guide(s)
Milestone

Comments

@calvinmetcalf
Copy link
Contributor

If I try to put array buffer into a db that has valueEncoding set to 'binary' I get an error about

message: "value cannot be an empty ArrayBuffer"
name: "WriteError"

as far as I can tell this is due to it being improperly turned into a Uint8 at some point, I managed to trace it to here where the array Buffer failed the Buffer.isBuffer test, but then when new Buffer(ArrayBuffer) is called it uses ArrayBuffer.prototype.length instead of ArrayBuffer.prototype.byteLength, so I'll likely have to open an issue there as well

@calvinmetcalf
Copy link
Contributor Author

feross/buffer/#39

@mcollina
Copy link
Member

mcollina commented Aug 5, 2014

I think all Level* is built on the assumption to use Buffer objects for binaries, and the use of ArrayBuffers, UInt8Array and bops is not really needed anymore thanks to the work of @feross.

Is it impractical to wrap your ArrayBuffer in an UInt8Array and then in a Buffer? What is your usecase?

We might want to add some docs on this in the README.

@calvinmetcalf
Copy link
Contributor Author

my use case involved passing buffers of images to socket.io on the server,
which came over as array buffers on the client side and using the array
buffer to build a blob uri that could be then passed to an image url. I
had wanted to use leveljs for some client side caching so I would prefer to
not have to wrap it in too much stuff that I then need to take off of it,
there where a few other issues involved

@feross's buffer silently turns arraybuffers to 0 length UInt8Arrays which
is unexpected but is also what node does, and there was a bug with levelup
that always returned buffers as strings.

On Tue, Aug 5, 2014 at 3:55 AM, Matteo Collina notifications@github.com
wrote:

I think all Level* is built on the assumption to use Buffer objects for
binaries, and the use of ArrayBuffers, UInt8Array and bops is not really
needed anymore thanks to the work of @feross https://github.com/feross.

Is it impractical to wrap your ArrayBuffer in an UInt8Array and then in a
Buffer? What is your usecase?

We might want to add some docs on this in the README.


Reply to this email directly or view it on GitHub
#34 (comment).

-Calvin W. Metcalf

@mcollina
Copy link
Member

mcollina commented Aug 5, 2014

I don't get if this bug still belongs here, and in case how we should address it :(.

@calvinmetcalf
Copy link
Contributor Author

One thing that would be helpful could be some custom data types, e.g array
buffer or blob (blob url maybe) that can efficiency save those types.
On Aug 5, 2014 5:47 AM, "Matteo Collina" notifications@github.com wrote:

I don't get if this bug still belongs here, and in case how we should
address it :(.


Reply to this email directly or view it on GitHub
#34 (comment).

@feross
Copy link

feross commented Aug 5, 2014

@calvinmetcalf I think the correct way to solve this issue is to just wrap the ArrayBuffers you're getting from your server with a Uint8Array and Buffer. It's just: new Buffer(new Uint8Array(arraybuffer) This is super lightweight and fast. You really don't have to worry about perf.

Then when you get the data out of level.js, you'll get a Buffer. If you need the data as a Uint8Array, you can just use the Buffer as-is, like this: buffer because the "Buffer" is actually just a Uint8Array with some added methods attached to the instance. If you need the data as an ArrayBuffer, just do: buffer.buffer. Simple.

@vweevers
Copy link
Member

@ralphtheninja following what @feross said, I think we should remove ArrayBuffer support. To be precise, remove this:

https://github.com/Level/level.js/blob/8c9f713e8db7e0383a733504c5996e507ae7fff5/index.js#L70-L71

And same for the _serializeKey logic added in #83.

@ralphtheninja
Copy link
Member

That should be tagged UPGRADING.md?

@ralphtheninja
Copy link
Member

@vweevers We can take @feross comment and add to UPGRADING.md as well.

@vweevers vweevers added this to the v3 milestone May 24, 2018
@vweevers vweevers added the upgrade guide Improvements or additions to upgrade guide(s) label May 24, 2018
@feross
Copy link

feross commented May 25, 2018

Btw, my comment was from 2014! I think Node.js is moving towards supporting Uint8Array and ArrayBuffer in addition to Buffer for built-in modules like fs, etc. Just FYI

@vweevers
Copy link
Member

Interesting! I guess we might restore ArrayBuffer support at some point, but the Level ecosystem is pretty much Buffer-only so for now we're good.

@feross
Copy link

feross commented May 25, 2018

Sounds good, consistency is more important!

@vweevers
Copy link
Member

FTR, we actually do support ArrayBuffer. I did not intend it but lol it just works 😄.

You can put an ArrayBuffer and it will come back as a Buffer by default, or an ArrayBuffer if you do get(key, { asBuffer: false }). When iterating binary keys, they will come back as Buffer by default, or an ArrayBuffer if you do iterator({ keyAsBuffer: false }).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upgrade guide Improvements or additions to upgrade guide(s)
Projects
None yet
Development

No branches or pull requests

5 participants