0.9 WIP

[rvagg]

Contains #128 by Dominic, a .jshintrc update and some README updates. Will probably also include #127 by @mcollina.

I'm also considering removing leveldown from the deps list for this release too, it's a big step though, but I haven't heard any negative feedback from #119 as long as we keep the _level_ package up to date and available for people wanting a fully bundled option. There's more activity now implementing LevelDOWN compatible interfaces, particularly by @maxogden and @No9, so it's making more sense to do this now.

[rvagg]

and, if I can actually find the time, I'd really like to fix #114 / Level/leveldown#32 too, it's a bit of a mess and I keep seeing people get tripped up by trying to call close() too early because they don't fully understand async.

[rvagg]

I'd like some feedback & help with 0efdc4c, it's the big shift away from LevelDOWN as a strict dependency. It's not require()d when needed and if you pass in a 'db' field in your options and don't use levelup.destroy() or levelup.repair() then it won't ever be needed.

There is a message in the throw that occurs if it can't be found which I think should make it clear to people who don't read the docs properly (most of us).

I've also renamed the instance variable _db to db to make it clear that you're allowed to access it directly to do implementation specific things on it. I'm tempted to remove approximateSize(), repair() and destroy() from LevelUP because they are very LevelDB-specific and other implementations just have to ignore them at the moment.

The real problem I have here is the lack of version pinning. Since we don't have an 'optionalPeerDependencies' field (/cc @domenic) we can't use npm to make sure that the right version of LevelDOWN is included with the current install of LevelUP. The best solution I can think of is checking require('leveldown/package').version for an acceptable version, but that'd mean we need to store metadata about what version is acceptable, perhaps that can go back in a new field in LevelUP's package.json, but then do we need to pull in semver to check it for us? It's a bit of a mess but the API is very tightly coupled between the two packages that having the wrong version of LevelDOWN for the version of LevelUP that you're using could do crazy and unexpected things and be difficult to debug for the user if there is a problem.

@maxogden you can use "git://github.com/rvagg/node-levelup.git#0.9-wip" as in your browser stuff now and give it a spin, pass in level.js as the 'db' option: var db = levelup({ db: require('level.js') })--as long as require('level.js')(location) is your db factory signature.

(btw, there was a reorg in open(), the diff there is mostly noise).

[rvagg]

See latest commit, 67ecfa5, for a semver test on leveldown. If LevelDOWN is required (i.e. a 'db' option hasn't been passed then it'll try and require() it, but it'll also try a semver match against the version matcher that LevelUP has in its devDependencies. So, we can provide two kinds of errors:

1. LevelDOWN can't be found, please install it
2. LevelDOWN is found but is the wrong version, please up/downgrade it

At this stage the LevelDOWN API is unstable enough that I think that it's important to be careful about version matching.

Thoughts?

[rvagg]

Oh, and I've added the "browser" section to package.json, can you browserify people double-check that for me please?

[juliangruber]

looks good to me

[No9]

We should pull browserify and the shim from leveldown when this lands then

[rvagg]

I've merged in @juliangruber chained batch work from #137 that exposes LevelDOWN's direct implementation of LevelDB's Batch operator. So now when you call batch() with no arguments you get a Batch object that you can use to put(), del() and write(). I also added clear() which is an additional LevelDB operation. I also added some tests similar to those in AbstractLevelDOWN except that they aren't quite as strict and they expect different error types.

AbstractLevelDOWN now has a matching implementation that works; MemDOWN uses the latest now and it works nicely with chained batch operations. LevelDOWN is up to date too with a new @0.3.1 release (I discovered after I released @0.3.0 that if you submit an empty batch it can segfault so I added simple protections for this case and released a patch version).

LevelDOWN also has @mcollina's change to iterator errors; next() and end() return errors on their callbacks (where they have them) rather than throwing. This should make for a much more pleasant experience with early db.close() operations (because an early close will end your iterators for you so you may get an error on a next() or end() that you call after a close()).

I think the only thing left to do is for anyone that's interested to review this new work and then we can release 0.9. I'm still hesitant because of the userland breakage that's going to happen when LevelDOWN is suddenly not bundled with LevelUP, but a note at the top of the README will hopefully help (still needs doing).

[brycebaril]

Browserify chokes here -- it doesn't seem to auto-find the ".json" suffix. I had to add a .json to make it happy.

[juliangruber]

fixed

[rvagg]

I'm wavering again on the new batch() syntax (sorry for being a bit fickle on this @juliangruber), my primary concern at the moment is extensibility.

Does the inclusion of this new syntax make it significantly more difficult to extend LevelUP?

@dominictarr have you looked at it at all and thought about how this might impact on hooks and sublevel? In its current form, it'll likely require intercepting the creation of the Batch object when batch() is called and monkeypatching write() calls and accessing this.ops.

[dominictarr]

@rvagg I'm in support of this api. It is actually quite close to how I'm extending batch already, for example, in level-hooks calling the hook for each op (rather than the batch as a whole).

[rvagg]

I've added checks to disallow any operations on an existing Batch object that has already had write() called on it. This is done in LevelDOWN and also tested in the latest AbstractLevelDOWN; if it's implemented properly there then the various projects using AbstractLevelDOWN should have this functionality automatically.

I think we're probably ready to push a release here, I'm satisfied that the identified memory leaks in LevelDOWN are now taken care of, I'll wait for @kylegetson to give some feedback on Level/leveldown#44 though.

I also need to find time to do a blog post on the major change we're making for this release (removing the LevelDOWN dependency) and explain the wins that come from this. So y'all have a bit of time to review the changes in this branch before it gets published.

[rvagg]

Lots of README updates including notes about LevelDOWN being optional and the new chained batch stuff.

See here

Diff for just these changes can be found here.

I've also split out level to it's own repo.

[rvagg]

Post is up: http://r.va.gg/2013/05/levelup-v0.9-some-major-changes.html

I might try and publish 0.9 tomorrow some time unless something else comes up.

[rvagg]

About to release this but I should say here that I've deprecated direct access to some LevelDB-specific stuff:

• db.approximateSize() -> db.db.approximateSize() (same for the new getProperty() that comes with LevelDOWN@0.5.0)
• levelup.destroy() -> leveldown.destroy()
• levelup.repair() -> leveldown.repair()

They are still in the code and tests but the documentation talks about using LevelDOWN directly for these. It's unreasonable to expect non-LevelDB back-ends to implement these very specific functions and it's not hard to use LevelDOWN directly now.

This may also apply to snapshots when they make it in, but we can discuss that in another issue.