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

binary keyEncoding fails indirectly #48

Closed
timkuijsten opened this issue Dec 30, 2015 · 6 comments · Fixed by #83
Closed

binary keyEncoding fails indirectly #48

timkuijsten opened this issue Dec 30, 2015 · 6 comments · Fixed by #83

Comments

@timkuijsten
Copy link

Guys,

I've been trying to port my application from Node to the browser using level-browserify. My code depends heavily on binary keys but unfortunately all write operations fail with the error "DataError: Data provided to an operation does not meet requirements." (Fx 45.0a2). When looking up the specs binary keys are not part of IndexedDB 1.0 but they are going to be part of IndexedDB Second Edition.

I've patched IDBWrapper to map Uint8Array keys to plain arrays in the meantime, but I think the level-js package is a better place to do this, especially considering issue #46. I was wondering if a patch that handles binary keys would be welcome. Otherwise I would suggest at least a patch that throws if keyEncoding is binary. Or maybe if idb-blob-store is the future I better open this issue there?

@mcollina
Copy link
Member

idb-blob-store is not the future, using IndexedDB directly is. A patch that removes the IDBWrapper (and bump dependencies) is highly welcomed. We are all low on bandwidth, but we can help if you want to take the lead on this.

@timkuijsten
Copy link
Author

Ok, but what about a patch that converts binary keys to native Arrays as a stopgap while IndexedDB Second Edition is not there yet?

@mcollina
Copy link
Member

I'm 👍 for that on the IndexedDB patch, not on the current code, as I am not sure what IDBWrapper will think about it. Moreover, the current code is not based on the latest abstract-level-down/levelup, which had a massive change in how the keys/values are handled.

@jensarps
Copy link

jensarps commented Jan 4, 2016

Just a heads-up: IDBWrapper will probably not convert invalid keys such as ArrayBuffers (or views). The reasoning behind this is the following: Say IDBWrapper would do this; once venders implement ArrayBuffers as valid keys, IDBWrapper would need to do so as well, which would introduce a backwards-compat breaking change.

So, no support from IDBWrapper on this, sorry (IMHO, converting keys is an application responsibility, and not one of a storage engine anyways). You'd either need to do this before passing keys to IDBWrapper, or push on realizing #46.

@timkuijsten
Copy link
Author

I agree with jensarps that this should and can not be resolved in IDBWrapper. Especially when restoring values from IDB to a JS Object. I'm working on a patch* that works with binary keys in level.js.

@mcollina as jens noted, IDBWrapper does nothing special with any type of key. Binary keys in level.js should be perfectly compatible with that.

@vweevers
Copy link
Member

vweevers commented May 24, 2018

#83 adds support for binary keys. If the browser doesn't support it, level.js will stringify the keys. It is then up to the user to encode binary keys (using a higher-level layer such as encoding-down).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants