Level / level-browserify

No longer maintained: superseded by level v5.0.0.
MIT License
63 stars 16 forks source link

Remove leveldown #5

Closed mcollina closed 10 years ago

mcollina commented 10 years ago

In #4 @rvagg suggested that we remove leveldown from here.

The whole point of this module was to provide a level experience that worked both in the browser and on the server.

I'm ok to remove it, but then what's its purpose?

(I'm opening a new discussion as I'll release the bugfix asap)

rvagg commented 10 years ago

@mcollina sorry, if that's the purpose then that's cool; I thought the purpose was to provide a convenience bundling for the browser so you can just require('level-browserify') in the browser like you require('level') on the server.

mcollina commented 10 years ago

The purpose was supporting code that was browserify-friendly on both the server and the client.

Maybe we can improve the readme, to make it clear :).

Any other opinions on this?

JamesKyburz commented 10 years ago

+1 for adding to readme.

I like the ability to have code run on server and client. Removing leveldown would break that.

If leveldown is to be removed, I think level should be made browserifyable.

juliangruber commented 10 years ago

it makes most sense to me to use level.js in the browser and leveldown in node...however having to compile leveldown when you just want to browserify sucks

JamesKyburz commented 10 years ago

@juliangruber that's true!

What are your thoughts on a browserfyable level then?

Browser only: Use level-browserify Node & Browser: browserify level using level-browserify

rvagg commented 10 years ago

ftr I'm not opposed to @JamesKyburz's suggestion of sucking this into level since level.js adds minimal dependency overhead, unlike the reverse as @juliangruber is pointing out

juliangruber commented 10 years ago

Hm, basically I think, because of the way npm and browserify work, it doesn't ever make sense to have level.js and leveldown in the same module.

How often have you used that? At least I haven't. This module got 17 downloads last month (https://www.npmjs.org/package/level-browserify).

If you want to write a module that works in node and browser, simply require the consumer to pass in a levelup instance, modules shouldn't really depend on level anyways.

JamesKyburz commented 10 years ago

@juliangruber I would like the best of both worlds if possible, being too lazy to write if window... or have two db implentations.

Adding level-js to level would be a small dependency as @rvagg mentioned.

I agree that having a leveldown dependency in level-browserify if all you want is browser stuff is not optimal.

juliangruber commented 10 years ago

my point is that browser and node always have separate entry points, so you can instantiante the appropriate dbs there and pass to your library code that works with any leveldb

JamesKyburz commented 10 years ago

@juliangruber true

But then what' the purpose if this module.

Without using level or level-browserify we can already do the following : use level-packager passing leveldown for node & level-js for the browser.

juliangruber commented 10 years ago

Yeah, my thinking - with the way browserify/npm works - is this module isn't of any real use...I too thought it was a cool idea but at least I personally didn't ever see a need for it.

juliangruber commented 10 years ago

No other module depends on this one

juliangruber commented 10 years ago

So...we can just leave the way it is :D

mcollina commented 10 years ago

Nice discussion! I've written this module to use it inside LevelGraph, but I have still not finished the new version. I'm also thinking about not including levelup/leveldown/leveljs in there, and just require them as an argument.