search

No9 / localstorage-down

localstorage implementation of leveldown
MIT License
41 stars 13 forks source link

(#39) - catch exceptions, abstractify #50

Closed nolanlawson closed 10 years ago

nolanlawson commented 10 years ago

So this is a big commit, but it represents the direction I would like localstorage-down to go in: more modular, more easily separable into an abstract-keyvalue-down component and a core localstorage component.

Notice that everything that touches localStorage itself has been moved into localstorage-core. Also, all access has been made sequential and asynchronous, so in principle localstorage-core could be subbed out for any key-value storage system. (My sights are currently set on lawnchair and Amazon S3.)

As for this commit itself, if you want to test in PouchDB, you'll have to wait on pouchdb/pouchdb#2252. But I tested it, and it passes at 100%.

nolanlawson commented 10 years ago

Also for the record, LocalStorageCore is tiny.

Some other things to note:

nolanlawson commented 10 years ago

OK, so let's not merge this until pouchdb/pouchdb#2252 is fixed.

calvinmetcalf commented 10 years ago

the thing to worry about with process.next tick is using it recursively this is why setImmediate is preferred for cases like this, it might make sense to switch back to the next tick setup they previously had which favored setImmediate on uptodate node versions

nolanlawson commented 10 years ago

@calvinmetcalf Okay, I fixed the nextTick stuff as well as the unnecessary queue. This passes the beefy tests as well as in pouchdb at master, you just have to do:

npm link /path/to/localstorage-down
CLIENT=selenium:firefox ADAPTERS=localstorage npm test

I would accept a :+1: from you as an honorary maintainer of this repo (would make you an official maintainer, but I apparently don't have the privileges). Else if you're busy, then ping @adamshih.

No9 commented 10 years ago

@calvinmetcalf no probs adding you if you want

calvinmetcalf commented 10 years ago

Last suggestion, use inherits instead of requiring utils it browserified big

nolanlawson commented 10 years ago

@calvinmetcalf Whoa no kidding, thanks.

nolanlawson commented 10 years ago

@calvinmetcalf So I assume you're good with this PR then?

calvinmetcalf commented 10 years ago

Yup On May 25, 2014 8:57 PM, "Nolan Lawson" notifications@github.com wrote:

@calvinmetcalf https://github.com/calvinmetcalf So I assume you're good with this PR then?

— Reply to this email directly or view it on GitHubhttps://github.com/No9/localstorage-down/pull/50#issuecomment-44150565 .