genericize test suite to make sense in browser

[max-mapper]

this almost passes all the leveldown tests (except iterators, havent worked on those yet): https://github.com/maxogden/level.js

there are two fundamental differences though:

• indexeddb natively supports storing all JS data types (num, bool, string, typed arrays, array buffers etc). with leveldb its either a string of a buffer, and the test suite is currently set up to always expect either a string or a buffer, which means if you store a bool in the browser the test suite converts it to a string
• Buffer doesn't exist in the browser. instead of using buffer-browserify (which if fundamentally flawed IMO) i'd rather return ArrayBuffers as they are the equivalent primitive binary data type in browsers. the problem with this is that the test suite currently does a lot of Buffer.toString() checking but toString() returns '[object ArrayBuffer]' on ArrayBuffers. instead you have to do String.fromCharCode.apply(null, new Uint16Array(arraybuffer))

so, is it cool if I add a bunch of conditional browser specific stuff to the test suite? is there a better 'paradigm' we could use for return value checking?

[juliangruber]

1. Only store strings and buffers in the first place
2. We need a meta module that wraps Buffer and ArrayBuffer depending on the environment, so the tests stay small.

[max-mapper]

Is it okay to abstract away the ability to store native data types so that
browser leveldb can be compatible with server leveldb? seems like there
should be an option... or at least a warning that when a sync happens all
non string/buffer data may be converted to string/buffer

On Tue, Apr 30, 2013 at 12:03 PM, Julian Gruber notifications@github.comwrote:

1. Only store strings and buffers in the first place
2. We need a meta module that wraps Buffer and ArrayBuffer depending on
   the environment, so the tests stay small.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/2#issuecomment-17247551
.

[juliangruber]

I prefer having only strings and buffers...That could cause frustrations and unexpected results when having to sync. It would be cool if all leveldb implementations behaved the same, so we have a solid base.

[No9]

Agree with @juliangruber. There is a lot of code sitting above that could make assumptions on storage. I am not sure what having an option is going to by you as all the modules above are going to expect string buffer?
@juliangruber The meta module approach sound interesting are you thinking conditional requires?
@maxogden do you have more details on why Buffers are fundamentally flawed?

[max-mapper]

trying to make Buffer work in the browser doesn't make sense because there
is no equivalent API. You can either make it work and have it be really
slow because its actually just a regular Object underneath or you can use
ArrayBuffers and Typed Arrays and have it be fast but I don't think you
can get the same Buffer API on top of those (I could be wrong, though)

today I believe buffer-browserify is just a bunch of sugar on top of
Object. for my use case the whole point of using leveldb is to get fast
storage for binary data that works in node and browser and syncs between
both as well. the only way to do that is to store typed arrays in the
browser and buffers in node and convert between them when you sync

On Tue, Apr 30, 2013 at 1:24 PM, Anton Whalley notifications@github.comwrote:

Agree with @juliangruber https://github.com/juliangruber. There is a
lot of code sitting above that could make assumptions on storage. I am not
sure what having an option is going to by you as all the modules above are
going to expect string buffer?
@juliangruber https://github.com/juliangruber The meta module approach
sound interesting are you thinking conditional requires?
@maxogden https://github.com/maxogden do you have more details on why
Buffers are fundamentally flawed?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/2#issuecomment-17251850
.

[max-mapper]

more thoughts:

• in browser you can do binary transport with XHR and/or websockets. so you
  can read typed arrays from indexeddb, write them to a binary websocket and
  node will parse them as buffers
• someArrayBuffer.toString() === '[object ArrayBuffer'] but
  someNodeBuffer.toString() === 'buffer contents'. we could maybe introduce a
  higher level binary container? that seems silly. this is a more general
  question
• multilevel uses mux-demux which uses JSON which doesn't do binary. we
  could either use msgpack (more complicated) or @dominictarr could maybe
  write a binary version of mux-demux so it can be piped over a binary stream

On Tue, Apr 30, 2013 at 1:42 PM, Max Ogden max@maxogden.com wrote:

trying to make Buffer work in the browser doesn't make sense because there
is no equivalent API. You can either make it work and have it be really
slow because its actually just a regular Object underneath or you can use
ArrayBuffers and Typed Arrays and have it be fast but I don't think you
can get the same Buffer API on top of those (I could be wrong, though)

today I believe buffer-browserify is just a bunch of sugar on top of
Object. for my use case the whole point of using leveldb is to get fast
storage for binary data that works in node and browser and syncs between
both as well. the only way to do that is to store typed arrays in the
browser and buffers in node and convert between them when you sync

On Tue, Apr 30, 2013 at 1:24 PM, Anton Whalley notifications@github.comwrote:

Agree with @juliangruber https://github.com/juliangruber. There is a
lot of code sitting above that could make assumptions on storage. I am not
sure what having an option is going to by you as all the modules above are
going to expect string buffer?
@juliangruber https://github.com/juliangruber The meta module approach
sound interesting are you thinking conditional requires?
@maxogden https://github.com/maxogden do you have more details on why
Buffers are fundamentally flawed?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/2#issuecomment-17251850
.

[juliangruber]

@maxogden for the tests we just need new Buffer(string or array) and compare buf1 to buf2, if I remember correctly. I guess that will work with both implementations?

And I'm working on mux-demux + redis-protocol-stream btw.

[dominictarr]

@maxogden so this means that buffer[4] is out in the browser - you must use arraybuffer.get(4) or something?

[juliangruber]

yup I think that's the current api.

[rvagg]

LevelUP already does the conversion to/from Buffers and Strings under the assumption that that's all that LevelDOWN handles, even though LevelDOWN has a toString() conversion of any non String & Buffer types already. https://github.com/rvagg/node-levelup/blob/master/lib/util.js#L20
So even if you wanted to bypass the stringification-of-everything here then you'd have to also tackle it in LevelUP, which could get messy.

[max-mapper]

(re-commented because email reply broke formatting because github has a bug or something)

Here are my goals for a storage lib for browsers that syncs with leveldb in
node (through multilevel):

• Be as fast as possible
• Don't abstract away features of IndexedDB
• Use the leveldown test suite and sync with multilevel over either ascii
  or binary transports (websockets and xhr both have ascii/binary modes in
  browsers now)

So, if we use Buffer it throws away the speed of Typed Arrays. If we use
Typed Arrays it breaks the leveldown test suite because Buffers are fast in
node but slow in the browser.

One solution is that I can put if (process.browser) clauses into the
leveldown tests that are Buffer specific and have them expect
ArrayBuffers/Typed Arrays in client side JS. When a Typed Array gets
written to a binary websocket it will come out the other end in node as a
Buffer anyway

[rvagg]

For my part I'm OK with having branches in the tests to deal with browser environments as a separate case. I don't know much about browserify's Buffer implementation but from what I've heard it does sound like a poor choice. Although I wonder whether that means that this is something to be fixed there rather than here? Or is that impossible?

This will add complexity to LevelUP though but we can at least feature-detect on init and replace the default Buffer conversion functions with whatever's more appropriate.

[max-mapper]

Good question, there is toots/buffer-browserify#8 but it has no reply from the maintainer.

Here are the issues I'm aware of in wrapping ArrayBuffers in the Buffer API (could be more):

• ArrayBuffer.toString() and Buffer.toString() have different semantics
• Buffer[idx] works but with an ArrayBuffer you have to make a typed array first and call [idx] on that
• Buffers are basically a hybrid of ArrayBuffers + Typed Arrays so it is difficult to get all the features of either set into the other
• node can do buffer to typed array but you can't do typed array to buffer

[max-mapper]

also cc smart people for moar input @isaacs @thisandagain

[No9]

@maxogden on the indexeddb support
I think we need to be aware that Indexeddb is not the only storage solution that will be accessed though browser based javascript.

In mobile scenarios we are using localstorage because of poor mobile support for indexeddb and this suffices for certain use cases.
See http://caniuse.com/indexeddb vs http://caniuse.com/namevalue-storage

There is also work progressing on integrating IOS and Android ports of leveldb which will be accessed through browser based javascript via phonegap.

These storage engines will no doubt have features (more likely idiosyncrasies) that they will bring and I wouldn't expect abstract-leveldown to cater for them too much.
Similarly it would be incorrect for abstract-level to insist that they implement JSON types because they are being interacted with from a browser.

Maybe there is an elegant solution that can resolve this problem but I thought it would be better to air my concerns.

[max-mapper]

Agreed that IDB won't be the only backend, but it will probably be the only one where low level performance matters. I think the behavior of leveldown right now (only strings and buffers) is too limiting. If we allow other JS types to exist but still allow the simpler case of strings + buffers then we should be able to have the test suite support lots of use cases.

[dominictarr]

So, we just need leveldown to handle the encoding - or to expose some option so to tell levelup not to do anything before passing it to leveldown.

[max-mapper]

ok, level.js, abstract-leveldown and leveldown all pass the same tests. I decided to just roll with strings and binary for now and to add some browser specific tests for other data types later

[feross]

FYI @maxogden - I'm pretty sure native-buffer-browserify solves all the problems with buffer-browserify that you mentioned. I think it should be preferred over ArrayBuffer in browser modules because ArrayBuffer doesn't work with streams.

Once native-buffer-browserify becomes the default in browserify. Then doing new Buffer will return an augmented Uint8Array instance with all the methods from the Buffer API added as properties. Square brackets (buf[0]) work as expected. And it doesn't modify any prototypes.

Thoughts?

[max-mapper]

@feross sounds good. just curious as to what you mean by "ArrayBuffer doesn't work with streams." though?

[feross]

Streams expect Buffer or string arguments for writable.write, readable.push, etc. right? So if you try to do writable.write(arraybuffer) you'll get an error because Buffer.isBuffer(arraybuffer) is false.

[feross]

This has been my biggest annoyance with bops. Once you convert a module to use bops then all your buffers suddenly don't work with streams in the browser because they're not instanceof Buffer.

[dominictarr]

@feross hmm, we should probably have the browser-built in do Buffer.isBuffer(UInt8Array) === true
because bops is a much better approach for binary in the browser?

can you post a gist of something that seems reasonable but isn't working?

[mcollina]

All readable-stream works only in object mode in the browser with bops buffers.

[max-mapper]

for modules like jsonparse (used in JSONStream) that only use the angle bracket syntax passing in a Buffer in node works exactly the same as a Uint8Array.

if you want to use other types of typed arrays then native-buffer-browserify isn't ideal, but for the majority of use cases it looks like a nice solution.

I still don't really understand your grievances with bops + streams, I guess I haven't ever run into those issues. i can't think of a stream that I've used in a browser that rejects non Buffer instances

[dominictarr]

@maxogden @mcollina is probably talking about https://npm.im/readable-stream

core node has things like Buffer.isBuffer(b) checks, that break some things,
I'm guessing that readable-stream does something like that too.

[mcollina]

Thanks @dominictarr, that was what I was saying.

I'm guessing that if we want to be bops-compliant, we should have a fork for readable-stream to something like readable-stream-bops.

[feross]

@mcollina I was hoping that by backing the browserify Buffer with Uint8Array that we wouldn't need a bops-compatible stream fork. We can just use Buffer normally and browserify will do the rest of the work. It'll use native browser buffers automatically so it's just as fast as bops, but there's no need to port every module to use bops.

[mcollina]

@feross exactly.

The problem is we have tons of stuff built on bops and typed arrays. How can we support both styles in levelup (and others)?

[max-mapper]

Buffer-browserify might not make as much sense here, as indexeddb can natively store any JS type except Object. So if you give it a typed array it can store it without any serialization, but I'm not sure about what would happen if you gave it one of the hacked uint8arrays from buffer-browserify

Sent from my iPhone

On Nov 25, 2013, at 2:35 AM, Matteo Collina notifications@github.com wrote:

@feross exactly.

The problem is we have tons of stuff built on bops and typed arrays. How can we support both styles in levelup (and others)?

—
Reply to this email directly or view it on GitHub.

[feross]

Remember that the Buffer API has toArrayBuffer() coming in 0.11 which
native-buffer-browserify already supports. So if you want a pristine typed
array for indexeddb you can always do new Uint8Array(buf.toArrayBuffer())

Not sure what makes sense for level here, but in general I think it would
be really nice if we could use browserify's buffer and get all the benefits
of bops without needing to port modules over.

On Monday, November 25, 2013, Max Ogden wrote:

Buffer-browserify might not make as much sense here, as indexeddb can
natively store any JS type except Object. So if you give it a typed array
it can store it without any serialization, but I'm not sure about what
would happen if you gave it one of the hacked uint8arrays from
buffer-browserify

Sent from my iPhone

On Nov 25, 2013, at 2:35 AM, Matteo Collina <notifications@github.com<javascript:_e({}, 'cvml', 'notifications@github.com');>>
wrote:

@feross exactly.

The problem is we have tons of stuff built on bops and typed arrays. How
can we support both styles in levelup (and others)?

—
Reply to this email directly or view it on GitHub.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/2#issuecomment-29219393
.

Feross
Blog http://feross.org | StudyNotes http://studynotes.org |
PeerCDNhttp://peercdn.com/

[feross]

@mcollina:

The problem is we have tons of stuff built on bops and typed arrays. How can we support both styles in levelup (and others)?

It's actually pretty easy to support both styles going forward. The Buffer constructor in native-buffer-browserify supports copying from arrayish objects, just like in the node API, so if you have a buffer that you got from bops you can convert to Buffer like this:

var bopsBuffer = bops.from('hey there!') // returns Uint8Array
var realBuffer = new Buffer(bopsBuffer) // returns Uint8Array augmented with Buffer methods, treat it like a node Buffer!

[feross]

For any library that needs to work with the streams2 implementation in browserify, it's important to use a real Buffer (or the Uint8Array/Buffer hybrid that browserify will return) and not a plain Uint8Array returned from bops, or else the stream will reject it because Buffer.isBuffer returns false.

If you really want to keep using bops with streams2, setting objectMode: true on the stream will work, though it changes the behavior of the stream a bit.

[ghost]

browserify v3 is very soon going to start using Uint8Arrays for Buffer by way of native-buffer-browserify.