+1 on this from me

location is an awkward quirk of history that we still haven't managed to get away from, perhaps it's time we completely ripped leveldown-by-default out of levelup and left that job to the level package, that way we don't need to do any initialisation or pass anything to a new leveldown object, we can just require a leveldown be passed in here.

For my own use, I almost always use level when I just want a vanilla leveldown+levelup setup.

how does this handle open/closed state. I think the reason that levelup expects a constructor function is so that it can manage open state, so that it's possible to get a sync open ui. Does this still work if you pass in an already opened leveldown?

👍

👍 for me.

@dominictarr: that does seem to be a problem:

var levelup = require('./');
var leveldown = require('leveldown');

var rows = [
    { type: 'put', key: 'a', value: 555 },
    { type: 'put', key: 'b', value: [ 3, 4, 5 ] },
    { type: 'put', key: 'c', value: { x: 7, y: 8 } }
];

var down = leveldown('/tmp/cool.db');
down.open(function () {
    console.log('open!');

    var up = levelup(down, { valueEncoding: 'json' });
    up.batch(rows, function (err) {
        if (err) return console.error(err);
        up.createReadStream().on('data', console.log);
    });
});

gives this output:

 $ node ex.js
open!

events.js:72
        throw er; // Unhandled 'error' event
              ^
OpenError: IO error: lock /tmp/cool.db/LOCK: already held by process
    at /home/substack/projects/node-levelup/lib/levelup.js:129:34

This patch seems to fix the lock error above, but I'm not sure if there are other implications:

From 39784034c68f561b88fc0ba01a4c1c1a4d3daa9e Mon Sep 17 00:00:00 2001
From: James Halliday <mail@substack.net>
Date: Tue, 13 Jan 2015 00:39:53 -0800
Subject: [PATCH] don't try to open a leveldown

---
 lib/levelup.js |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/lib/levelup.js b/lib/levelup.js
index e48e31d..10783b9 100644
--- a/lib/levelup.js
+++ b/lib/levelup.js
@@ -94,7 +94,12 @@ function LevelUP (location, options, callback) {
   // set this.location as enumerable but not configurable or writable
   prr(this, 'location', location, 'e')

-  this.open(callback)
+  if (down) {
+    this._status = 'open'
+    this.db = down
+  } else {
+    this.open(callback)
+  }
 }

 inherits(LevelUP, EventEmitter)
-- 
1.7.10.4

The documentation in leveldown is quite clear that leveldown does not track the open states, so it might be good to add a reminder to the levelup docs that any leveldown that is passed in should already be open.

the cleaner solution seems to be to add a .open state to leveldown, so you can pass open and closed leveldown instances. however given our ecosystem, the approach above also sounds good to me

I agree with julian here, to do this properly we need a state property on leveldown.

+1 Let's get this into levelup 1.0!

im +1 on this if it wont block a better implementation (keeping the open/closed state on leveldown). @kesla you know the implications here?

@jcrugzz sorry no

Further out, I'm not really sure what the location property is doing in there at all or what it's for. It seems like handling a location path should be leveldown's job, not levelup. Plus there are more ways to specify a "location" than a simple string value, which leveldowns will be better at providing given their unique specializations.

I agree. Lets get rid of location in a separate PR.

Moved this to new PR. Closing. Thank you @substack !

And +1 for having .open state in leveldown