Make LevelDOWN an optional dependency

[rvagg]

Consider this a discussion pull request.

We now have a 'db' option on init that can swap out LevelDOWN with a suitable replacement (e.g. MemDOWN or other via Abstract LevelDOWN). We'll hopefully one day have pure JS implementation(s) of LevelDB available to swap in.

So, it would be nice to not have to pull in a compiled dependency if you weren't going to need it.

This PR makes it an optional dependency but pins the version with a 'peerDependency'. It would just add complication to install instructions because we'd have to instruct people to npm install levelup leveldown but hopefully the thrown Error would help with that.

Thoughts?

[Raynos]

@rvagg npm installs peer deps automatically if one isn't already there

[rvagg]

@Raynos aye, you're right! So ignoring the peerDependencies dep, I'd still like discussion about detaching LevelUP from LevelDOWN.

[Raynos]

@rvagg +1 for me. I want to to implement the leveldown API for indexeddb again at some point.

The problem with leveldown as a peer dep is I dont want levelup to have binary dependencies when used with indexeddb.

[rvagg]

I guess if we can't pin version with 'peerDependencies' then we can't rely on npm to sort out versions for us which could be a problem since it's important that the LevelDOWN API is correct for the version of LevelUP.

Of course we could introduce a third package that does the bundling, or push the LevelUP stuff into the third package and expose it here along with the LevelDOWN dependency. Perhaps that's getting a bit out of hand though.

[Raynos]

@rvagg we could fix peer deps to not auto install it.

[rvagg]

@domenic do you have thoughts on changing "peerDependencies" behaviour so that it doesn't auto-install when the dependency doesn't exist? All we want to do is pin to a particular version of the dependency if and only if it's installed. "optionalDependencies" would make sense for this if it didn't already have a use-case (does anyone actually use it though?)

[Raynos]

@rvagg we can just peer depend on abstract leveldown. This at least means we can match the versions of leveldown & abstract leveldown

[Raynos]

Also! If leveldown peer deps on abstract leveldown then we get an npm error if the versions of leveldown & levelup you use dont work together!

[domenic]

Yeah, there were some musings on "optionalPeerDependencies". Sounds like they're gathering support, hmm.

[rvagg]

@Raynos true, then we can just treat AbstractLevelDOWN as the API spec.

Would like feedback from others' regarding usability and if you think this might create problems from people who just want to use leveldb and don't want to have to mess with understanding the dependencies required.

[juliangruber]

Would AbstractLevelDOWN depend on LevelDOWN? If yes, we're not gaining anything, right? And if not, that won't help with the simple npm install levelup.

As soon as we have LevelJS this is a no-brainer, just depend LevelJS and use LevelDOWN only when installed (like with redis and hiredis).

For now we can either:

• add db.version to LevelDOWN for version safety and make LevelUP throw an Error if no db has been passed and LevelDOWN isn't installed. This way users have to do npm install leveldown levelup.
• not make levelUp depend on levelDOWN and create a meta package level that depends on LevelUp and LevelDOWN to combine them.

I quite like the 2nd option.

[rvagg]

AbstractLevelDOWN would be a peerDependency for both LevelUP & LevelDOWN, so its version would define the LevelDOWN API version. When you npm install levelup leveldown the version of AbstractLevelDOWN shared by both would have to match up for it to be acceptable.
Of course then we (I) have to start caring more about which version ranges are acceptable rather than individual versions but I think that should be ok since I don't break API at patch-level.

[dominictarr]

The only sane way to do this is to republish levelup into a new module without leveldown, and then publish a levelup-down module (possibly changing names) or... publish levelup again, without leveldown.

So, either have levelup, leveldown stand alone modules, and a bundling module, or levelup-without-leveldown
none of these feel ideal... but the alternative is adding a feature to npm.

Heh, in a way, levelup is becoming a "template" and you inject an level-down parameter.
We've all come a long way if we have stretched npm to maybe needing to do this.

[juliangruber]

@dominictarr that's what I propose level for, that name is free!

[rvagg]

whoa! these are competitive times for npm names, careful announcing something like that in public!

[dominictarr]

There arn't many 5 letter names left. That is a sign!

[juliangruber]

@rvagg haha you already took it :)

[rvagg]

tis here.

So, it sounds like the way forward is to detach LevelDOWN from LevelUP and use this new package to bundle them together. Currently it's simply an exports = require('levelup') which ought to be enough I think.

And, I still like the idea of pinning AbstractLevelDOWN as a peerDependency to both LevelUP and LevelDOWN to keep them in sync.