Level / community

Discussion, support and common information for projects in the community.
MIT License
43 stars 15 forks source link

My approach to plugins #51

Open rvagg opened 11 years ago

rvagg commented 11 years ago

Posting in a new issue instead of #68 or #80 because this is about a specific implementation that I'd like comment on.

The LevelUP pluggability branch contains an extension to 0.6.x that uses externr to expose some basic extension points (more are possible of course). I've put an example in that branch here that places prefixes on keys (and removes them on the way out), and also prevents reading of certain other keys, purely to provide an example.

The externr approach is quite different from the level-hooks approach so I don't imagine this to be non-controversial. My hope is that the two approaches can coexist and perhaps leverage off each other.

While externr is pretty fast for the 'noop' case where you don't have a plugin operating on a given extension point, my guess is that it'll undo the performance gains in #90 (which haven't made it into a release yet fyi).

The way LevelUP extensions work is with a "use" call or option. There is a global levelup.use(plugin) that you can register plugins with any LevelUP instance created after that point. There is a "use" property on the options argument to levelup() when you're making a new instance, the property can point to a single plugin or an array of plugins. That instance will then use those plugins. Each instance will also expose a .use() method that can take single or arrays of plugins, so you could add plugins after an instance is created.

Plugins are simply objects whose keys are the extension points it wishes to inject itself in to. The values are functions that do the work. See the example linked to above to see what I mean.

The LevelDOWN plugins branch contains additions the implement a basic plugin system for the native layer by way of providing a Plugin class that can be extended. So far it has only one extension point, an Init(database) method that passes a newly created LevelDOWN database instance (i.e., when you call leveldown(location)). Plugins can then do what they like on the database object, mostly just adding methods or replacing existing ones I suspect. But I imagine also being able to offer a mechanism to insert a particular LevelDB comparator if you need your db sorted in a particular way. Or even more advanced, a replacement LevelDB filter if you have something more efficient than the default bloom filter.

Currently the way you put a plugin in LevelDOWN is with a global leveldown._registerPlugin(location) call, where location is the path to a .node object file it can dlopen(). Once loaded, the plugin is placed into a list of plugins and when needed the list is iterated over and each plugin has the appropriate method invoked (currently just Init() on instance creation).

Extending LevelDOWN is quite tricky, I'm not aware of any other native build offering plugins. So there are a few challenges. Currently there's an npm issue that's preventing it from being a seamless thing.

My working example is a range delete which I decided could be implemented outside of LevelUP/LevelDOWN and offered as a plugin. @Raynos has level-delete-range but when you have to do individual deletes on callbacks from an iterator it's majorly inefficient; you really want to be doing bulk deletes in one go, native & async.

Enter Downer RangeDel. I've exposed a .use() function on the exports so you have to explicitly run that to make it inject itself into the nearest leveldown it can find (hopefully there's just one, peerDependencies should help with that). When a LevelDOWN instance is created, it attaches a .rangeDel() method to the object. The plugin is able to reuse a lot of the existing LevelDOWN code for iterators so the .rangeDel() method has an almost identical options signature to a .readStream() in LevelUP. Doing the actual delete is a simple 5-line job but it's efficient and done in one go with no callback until it's completed.

Then, to make that available to LevelUP, I have Upper RangeDel. It has downer-rangedel as a dependency so you only need to load it alongside levelup to get going. I've also exposed a .use() method there so you have to explicitly invoke it too. It'll inject itself globally into LevelUP so it'll run in every LevelUP instance you create (but, you could opt out of global and levelup({ use: require('upper-rangedel') }) for an individual instance.

The code for this should be a bit easier to understand cause it's all JS. The plugin simply extends the "constructor" of each LevelUP instance and passes the call down to this._db.rangeDel() where it expects that Downer RangeDel has put a method. I've also added some arguments checking and mirroed the deferred-open functionality in the rest of LevelUP so you could do something like: levelup('foo.db').rangeDel() and it should work.

To show it in action, I have an example of Upper RangeDel in work. It uses "dev" tagged levelup and leveldown releases in npm as this is not available in current "latest" releases.

package.json

{
  "name": "example",
  "version": "0.0.0",
  "main": "index.js",
  "dependencies": {
    "levelup": "~0.7.0-b02",
    "upper-rangedel": "0.0.1"
  }
}

index.js

require('upper-rangedel').use()

var levelup = require('levelup')
  , db = levelup('/tmp/foo.db')
  , data = [
        { type: 'put', key: 'α', value: 'alpha' }
      , { type: 'put', key: 'β', value: 'beta' }
      , { type: 'put', key: 'γ', value: 'gamma' }
      , { type: 'put', key: 'δ', value: 'delta' }
      , { type: 'put', key: 'ε', value: 'epsilon' }
    ]
  , printdb = function (callback) {
      db.readStream()
        .on('data', console.log)
        .on('close', callback)
        .on('error', callback)
    }

db.batch(data, function (err) {
  if (err) throw err
  console.log('INITIAL DATABASE CONTENTS:')
  printdb(function (err) {
    if (err) throw err
    db.rangeDel({ start: 'β', limit: 3 }, function (err) {
      if (err) throw err
      console.log('\nDATABASE CONTENTS AFTER rangeDel({ start: \'β\', limit: 3 }):')
      printdb(function (err) {
        if (err) throw err
        console.log('\nDone')
      })
    })
  })
})

Output

INITIAL DATABASE CONTENTS:
{ key: 'α', value: 'alpha' }
{ key: 'β', value: 'beta' }
{ key: 'γ', value: 'gamma' }
{ key: 'δ', value: 'delta' }
{ key: 'ε', value: 'epsilon' }

DATABASE CONTENTS AFTER rangeDel({ start: 'β', limit: 3 }):
{ key: 'α', value: 'alpha' }
{ key: 'ε', value: 'epsilon' }

Done

You can run this now, but you'll have to do a second npm install after the first one finishes with a failure; this is something we'll need to overcome in npm or with some crazy hackery with gyp or npm pre/postinstalls.

This obviously needs more polish and thought before its production ready, but I also want to make sure I have some kind of agreement on the approach before I push ahead. So I need your thoughts!

Raynos commented 11 years ago

Why all this complex machinery when you can call rangeDel(db, ...). Is there a way to write a C++ extension that plays nicely without leveldown with global pollution?

ghost commented 11 years ago

Its good to see a discussion like this to allow plugins architecture to be performant.

Is it possible that the advantages and disadvantages of the JS versus c++ approach be listed to provide more clarity ?

For my own usage i favour a pure JS solution to allow easy of developing plugins. I also favour that the actual CRUD's do not need to have any knowledge of the plugins. to do this i would have thought that a serious of base handlers be available that get called by levelup, which they runs the plugins automagically on the query.

rvagg commented 11 years ago

@Raynos I'm trying to come up with an approach that makes a more consistent plugin ecosystem so that users don't have to do something new & different for each thing they want to bolt on. I know you're comfortable plugging many tiny bits together to form a complete, customised whole but I'd like to make this as easy as possible for the average dev who doesn't have time to dig into the tiny details of each element they need in order to come up with a solution that fits their problem. I'm only using .rangeDel() as a single example and sure, there's plenty of other ways to do this without the "machinery" inside LevelUP but that machinery should also allow many varied different plugins that modify behaviour or bolt on new functionality. For example: transparent namespacing of keys. Or: transparently inserting additional entries to form indexes for primary entries and adding a .getBy() method that can fetch by those indexed properties. Or: bypassing a proper .get() in favour of .batch(). These can all be achieved by other means but each would take a different approach and would require a different procedure to make it work and it'd be difficult to know if they could work together. My "machinery" should make these bits work together nicely and would provide a consistent interface to load and insert plugins to the core.

@Raynos can you clarify your question about C++ extensions and global pollution?

@gedw99 pure JS implementations are fine for most things that could conceivably extend LevelUP but there are some cases where that's not possible (e.g. comparators that adjust internal sorting in LevelDB) and other cases where the efficiency gain is significant, such as .rangeDel()--but if this is not going to be a common operation and your data set isn't large then doing it all in JS-land is fine. It's crossing the V8->native boundary that's costly, plus with .rangeDel() you have to go back and forth on that boundary and between threads for each entry whereas when you do it in C++ you do it in a single thread, a single async call and you don't actually need to pass anything across that boundary to get the job done.

rvagg commented 11 years ago

If anyone's interested in the speed impact of doing a native .rangeDel() vs doing it in JS-land, see simple benchmark for it here https://gist.github.com/rvagg/5116514 - results here:

Native .rangeDel() benchmark
Filling database
Starting .rangeDel()...
Native .rangeDel() on 100000 entries took 262 ms
JS rangeDel() benchmark
Filling database
Starting rangeDel()...
JS rangeDel() on 100000 entries took 1636 ms
Done
ghost commented 11 years ago

massive difference.

i guess then we have to have some plugins at c++ level. so for the c+ newbies then we need to be able to toggle it on and off easily ?

G

On 8 March 2013 14:45, Rod Vagg notifications@github.com wrote:

If anyone's interested in the speed impact of doing a native .rangeDel()vs doing it in JS-land, see simple benchmark for it here https://gist.github.com/rvagg/5116514 - results here:

Native .rangeDel() benchmark Filling database Starting .rangeDel()... Native .rangeDel() on 100000 entries took 262 ms JS rangeDel() benchmark Filling database Starting rangeDel()... JS rangeDel() on 100000 entries took 1636 ms Done

— Reply to this email directly or view it on GitHubhttps://github.com/rvagg/node-levelup/issues/92#issuecomment-14619731 .

Contact details: +49 1573 693 8595 (germany) +46 73 364 67 96 (sweden) skype: gedw99

rvagg commented 11 years ago

Well @gedw99, they wouldn't be enabled or even available by default. So far nobody is suggesting they actually get bundled with the core. So you'd have to explicitly install them with npm and either invoke them with their own custom magic (@Raynos' approach) or inject them with special, consistent LevelUP magic (my approach).

I keep on thinking up awesome native plugins now that it's (almost) possible. As I was doing those benchmarks, I included a countEntries() function that had to use a KeyStream to do the counting which is not cool. Why not have a plugin that does a simple .count() using iterators at the native level?

Of course I must not get too carried away here with C++, LevelJS awaits!

ghost commented 11 years ago

ok well NPM install is easy. I thought you were going to embed it wit the core.

god design

I was looking at the NodeJS of rails last iht. Its called Sails and acts as an ORM. It is very useful for the developer that just wants to get things done.

they have adapters for many differnt DB's and its a very smooth and easy architecture.

i reckon a plugin for them at the CORE level woudl be amazing

g

On 8 March 2013 15:06, Rod Vagg notifications@github.com wrote:

Well @gedw99 https://github.com/gedw99, they wouldn't be enabled or even available by default. So far nobody is suggesting they actually get bundled with the core. So you'd have to explicitly install them with npm and either invoke them with their own custom magic (@Raynoshttps://github.com/Raynos' approach) or inject them with special, consistent LevelUP magic (my approach).

I keep on thinking up awesome native plugins now that it's (almost) possible. As I was doing those benchmarks, I included a countEntries()function that had to use a KeyStream to do the counting which is not cool. Why not have a plugin that does a simple .count() using iterators at the native level?

Of course I must not get too carried away here with C++, LevelJS awaits!

— Reply to this email directly or view it on GitHubhttps://github.com/rvagg/node-levelup/issues/92#issuecomment-14621282 .

Contact details: +49 1573 693 8595 (germany) +46 73 364 67 96 (sweden) skype: gedw99

ghost commented 11 years ago

also plugins for GIS spatial stuff will be able to b put into the leveldown code when they shoudl be.

It will be really interesting to see how we can get master master replication going with a map reduce style of querying

g

On 8 March 2013 15:26, Ged Wed gedw99@gmail.com wrote:

ok well NPM install is easy. I thought you were going to embed it wit the core.

god design

I was looking at the NodeJS of rails last iht. Its called Sails and acts as an ORM. It is very useful for the developer that just wants to get things done.

they have adapters for many differnt DB's and its a very smooth and easy architecture.

i reckon a plugin for them at the CORE level woudl be amazing

g

On 8 March 2013 15:06, Rod Vagg notifications@github.com wrote:

Well @gedw99 https://github.com/gedw99, they wouldn't be enabled or even available by default. So far nobody is suggesting they actually get bundled with the core. So you'd have to explicitly install them with npm and either invoke them with their own custom magic (@Raynoshttps://github.com/Raynos' approach) or inject them with special, consistent LevelUP magic (my approach).

I keep on thinking up awesome native plugins now that it's (almost) possible. As I was doing those benchmarks, I included a countEntries()function that had to use a KeyStream to do the counting which is not cool. Why not have a plugin that does a simple .count() using iterators at the native level?

Of course I must not get too carried away here with C++, LevelJS awaits!

— Reply to this email directly or view it on GitHubhttps://github.com/rvagg/node-levelup/issues/92#issuecomment-14621282 .

Contact details: +49 1573 693 8595 (germany) +46 73 364 67 96 (sweden) skype: gedw99

Contact details: +49 1573 693 8595 (germany) +46 73 364 67 96 (sweden) skype: gedw99

Raynos commented 11 years ago

@rvagg I have zero objection to the underlying machinery needed to standardize the authoring and creating of plugins from a module author point of view. I think all plugin authors should create them in a similar fashion.

I have an objection to the user facing API for people that use plugins

instead of

require('upper-rangedel').use()

db.delRange(...)

I'd be far nicer to have

var delRange = require('upper-rangedel')

delRange(db, ...)

The reasoning for this is that it's explicit and less magical plugins that mutate global db even if AT THE IMPLEMENTATION LEVEL plugins mutate the global db

rvagg commented 11 years ago

Right, I see what you mean by 'global' then. Btw, my implementation doesn't force mutating the global, you can do it only for specific instances too: var db = levelup('foo', { use: require('upper-rangedel') }) or, with an existing db: db.use(require('upper-rangedel')). The require('upper-rangedel').use() version is if you want it done on all instances automatically (which I suspect will be the preferred use-case for most people for a plugin like this).

And, re the LevelDOWN version, yes I think it might be possible to do your preferred approach there too rather than injecting directly into the instance. require('downer-rangedel')(this._db). I think it comes down to similar questions as in JS-land re which properties and methods you expose publicly. One nice feature of the externr approach is that you can expose any (or all) of that private junk we have in the closure that we use to create an hold new instances. So plugins can have special access to private properties that aren't available on the public API.

But I guess ultimately it comes down to preferences and I'd like to hear more voices on this topic before anything happens. I maintain that my approach provides more consistency and simplicity for end-users who just want to build stuff and not be too concerned with how LevelUP and it's ecosystem works. "Just give me a database with these features, I have awesome stuff to build".

dominictarr commented 11 years ago

This is interesting. When we originally discussed rangeDel I had assumed that it would have to be added to leveldown. Very interesting if they could be loosly coupled to the db.

In the case of something like rangeDel which just iterates over a range, doing deletes, couldn't you just pass the C object to the range delete plugin?

rangeDel = RangeDel(db) ?

passing leveldown to the plugin could be simpler here.

However, this issue is discussing two quite different things - C++ leveldown plugins, and particular extension points.

I think @Raynos' concern is valid, adding extensions willy nilly is gonna end in pain. I'm not sure that encouraging users to extend the base db object with their own stuff is such a good idea, I know from building those things that they are quite hard to get right. So intrinsically encouraging that stuff is asking for trouble (bugs)

I described the plugin approach I was using here, https://github.com/rvagg/node-levelup/wiki/Plugin-Pattern-Discussion, in brief, this worked my monkey-patching certain things, and then applying special rules on keys inserted into particular ranges.

It was possible to create all sorts of interesting extra features with that approach, but I ended up duplicating code for managing ranges and prefixes in every plugin, they where messy, and hard to perfect.

I have recently been working on a new approach, based on level-sublevel

As this issue is already covering several topics, I'll start a new issue.

dominictarr commented 11 years ago

my revised plugin approach: https://github.com/rvagg/node-levelup/issues/97

dominictarr commented 11 years ago

one thing I've needed to do is hook into a put or del call, decide to add something, and turn it into a batch. is it possible to do that with extenr?

rvagg commented 11 years ago

On your last question, this is from the externs_example.js, a simple, sneak plugin that cancels a put() if it doesn't like the look of your key.

   var sneakyextern = {
        put: function (key, options, valueEnc, callback, next) {
          if (key == 'foo2') {
            // bypass the real get() operation, jump straight to the user callback
            return callback()
          }
          // internal next() callback for the extern chain
          next(key, options, valueEnc, callback)
        }
    }

You'd just extend this a little and get this:

  var batchplugin = {
        put: function (key, options, valueEnc, callback, next) {
          this.batch([ { type: 'put', 
          if (key == 'foo2') {
            // bypass the real get() operation, jump straight to the user callback
            return callback()
          }
          // internal next() callback for the extern chain
          next(key, options, valueEnc, callback)
        }
    }

But I thought that a more interesting example would be in order so I created an indexing plugin using the externr approach. See https://github.com/rvagg/node-levelup/blob/pluggability/externs_example.js

The example stores this data (top npm packages):

        {  {
      name    : 'underscore'
    , author  : 'jashkenas'
    , version : '1.4.4'
    , url     : 'https://github.com/documentcloud/underscore'
  }
, {
      name    : 'async'
    , author  : 'caolan'
    , version : '0.2.6'
    , url     : 'https://github.com/caolan/async'
  }
, {
      name    : 'request'
    , author  : 'mikeal'
    , version : '2.14.0'
    , url     : 'https://github.com/mikeal/request'
  }
/* duplicate indexed properties are left as an exercise for the reader!
, {
      name    : 'coffee-script'
    , author  : 'jashkenas'
    , version : '1.6.1'
    , url     : 'https://github.com/jashkenas/coffee-script'
  }
*/
, {
      name    : 'express'
    , author  : 'tjholowaychuk'
    , version : '3.1.0'
    , url     : 'https://github.com/visionmedia/express'
  }
, {
      name    : 'optimist'
    , author  : 'substack'
    , version : '0.3.5'
    , url     : 'https://github.com/substack/node-optimist'
  }

and then it enables the database to be operated on like this, note the new .getBy() method:

// a standard get() on a primary entry
db.get('underscore', function (err, value) {
  if (err) throw err
  console.log('db.get("underscore") =', JSON.stringify(value))
})

// some gets by indexed properties
db.getBy('author', 'jashkenas', function (err, value) {
  if (err) throw err
  console.log('db.getBy("author", "jashkenas") =', JSON.stringify(value))
})

db.getBy('author', 'mikeal', function (err, value) {
  if (err) throw err
  console.log('db.getBy("author", "mikeal") =', JSON.stringify(value))
})

db.getBy('version', '0.2.6', function (err, value) {
  if (err) throw err
  console.log('db.getBy("version", "0.2.6") =', JSON.stringify(value))
})

producing this output:

db.get("underscore") = {"name":"underscore","author":"jashkenas","version":"1.4.4","url":"https://github.com/documentcloud/underscore"}
db.getBy("author", "jashkenas") = {"name":"underscore","author":"jashkenas","version":"1.4.4","url":"https://github.com/documentcloud/underscore"}
db.getBy("author", "mikeal") = {"name":"request","author":"mikeal","version":"2.14.0","url":"https://github.com/mikeal/request"}
db.getBy("version", "0.2.6") = {"name":"async","author":"caolan","version":"0.2.6","url":"https://github.com/caolan/async"}

tell me that's not awesome!

rvagg commented 11 years ago

and in that example's case, the plugin is all of 28 lines without comments; of course it's incomplete but it works nicely as an example of what's possible.

rvagg commented 11 years ago

Sorry. That example is here: https://github.com/rvagg/node-levelup/blob/pluggability/externs_example2.js

The db init code is simply this:


  , db = levelup('/tmp/indexer.db', {
        // inject the plugin
        use             : indexer
        // our plugin reads this option
      , indexProperties : [ 'author', 'version' ]
      , keyEncoding     : 'utf8'
      , valueEncoding   : 'json'
    })

Note how it is able to take options of its own.

Raynos commented 11 years ago

Why can we not have simple things like

var getBy = indexer(db, ["author", "version"]

// no magic. Just functions
getBy("author", "jashkenas", function () { ... })

I think we should definitely have low level hooks & extensions points for leveldb MODULE authors. But those things are implementation details.

But we shouldn't encourage a magical use and just drop all the properties in one big massive config hash approach.

ralphtheninja commented 11 years ago

@rvagg Very nice! But I think @Raynos has a point. Lets say I'm completely new to LevelUP and want to use this new cool indexing module that I heard so much about. It's hard to see the connection between the values in the options hash and the actual plugin, and if there are more plugins used, then this config is going to be massive. What if some other plugin also uses the 'indexProperties' property?

rvagg commented 11 years ago

@raynos I have a feeling we're talking about slightly different things here. This is only about module authors, being able to make modules that extend levelup in a consistent way that provide module users with a simple and consistent approach to using plugins together.

There's no massive config hash, I don't know where you're seeing that. Plugins are objects that expose the extension points they care about, they don't get munged in together into some big hash. Plugin extension points extend from each other so when you add a plugin with the externr mechanism they form a chain with one executing after the other for the same extension point. So, when you have an db instance that has an "index" plugin, it can also have a totally different plugin working at the same time and the "indexer" doesn't have to care about functionality beyond its own stuff--in your approach, how do you extend on top of your "indexer" without reimplementing large chunks of levelup or saying that it can't be extended itself so you can only have indexer and nothing else significant on it as well?

I'd appreciate it if you had a look at externr and try to understand what it's doing rather than dismissing this approach so easily.

rvagg commented 11 years ago

@ralphtheninja the indexProperties isn't important, it could just as easily have worked like this:

var db = levelup('indexed.db', {
    use           : indexer([ 'author', 'version' ])
  , keyEncoding   : 'utf8'
  , valueEncoding : 'json'
})

It's the use here that's important, it can be an array too:

var db = levelup('indexed.db', {
    use           : [ geospatial, indexer([ 'author', 'version' ]), rangeDel ]
})

Or, if that's not to your taste:

var db = levelup('indexed.db')
db.use([ geospatial, indexer([ 'author', 'version' ]), rangeDel ])

or

var db = levelup('indexed.db')
db.use(require('levelup-geospatial'))
db.use(require('levelup-indexer')([ 'author', 'version' ]))
db.use(require('upper-rangedel'))

The point being, these all have the same mechanism for working inside of levelup and can play happily together and the user doesn't have to get lost in this kind of cruft:

var db = levelup('indexed.db')
db = require('levelup-geospatial')(db)
var indexedDb = require('levelup-indexer')(db, [ 'author', 'version' ]) // ooops, can I extend the geospatial version of the db?
var rangeDel = require('upper-rangedel')
rangeDel(db) // which instance of `db` am I working on here? can it handle this?
ralphtheninja commented 11 years ago

I like this version the most:

var db = levelup('indexed.db')
db.use(require('levelup-geospatial'))
db.use(require('levelup-indexer')([ 'author', 'version' ]))
db.use(require('upper-rangedel'))

How about not having to use require?

var db = levelup('indexed.db')
db.use('levelup-geospatial')
db.use('levelup-indexer', [ 'author', 'version' ])
db.use('upper-rangedel')
Raynos commented 11 years ago

@rvagg

var db = levelup('indexed.db')
var getBy = require('levelup-indexer')(db, [ 'author', 'version' ]) 
getBy("author", "raynos", cb); /* will just work */
var rangeDel = require('upper-rangedel')
rangeDel(db, { /* some range */ }) /* this will just work */

I simply think this api is nicer then the .use() api. That's all. Heck for all I care all those functions can do db._use under the hood.

I prefer the attitude of "we have a module system and a bunch of leveldb compatible modules" versus "hey guys we have a plugin system. you can write plugins for our platform"

The former says we write commonJS modules that work with leveldb and follow the node style, if you know how to write modules, you know how to make a modular database.

The latter says come use our special plugin system that's different from every other plugin system for every other framework. It's kind of like node based commonJS modules, but not!

ghost commented 11 years ago

the standard commonJS way of doing it is also less limitting i feel.

plugin systems MUST get the API right for all possible intended and unintended use cases. This is hard to get right.

On 9 March 2013 23:27, Raynos notifications@github.com wrote:

@rvagg https://github.com/rvagg

var db = levelup('indexed.db')var getBy = require('levelup-indexer')(db, [ 'author', 'version' ]) getBy("author", "raynos", cb); /* will just work /var rangeDel = require('upper-rangedel')rangeDel(db, { / some range / }) / this will just work */

I simply think this api is nicer then the .use() api. That's all. Heck for all I care all those functions can do db._use under the hood.

I prefer the attitude of "we have a module system and a bunch of leveldb compatible modules" versus "hey guys we have a plugin system. you can write plugins for our platform"

The former says we write commonJS modules that work with leveldb and follow the node style, if you know how to write modules, you know how to make a modular database.

The latter says come use our special plugin system that's different from every other plugin system for every other framework. It's kind of like node based commonJS modules, but not!

— Reply to this email directly or view it on GitHubhttps://github.com/rvagg/node-levelup/issues/92#issuecomment-14672039 .

Contact details: +49 1573 693 8595 (germany) +46 73 364 67 96 (sweden) skype: gedw99

Raynos commented 11 years ago

Also if the plugin API is only used by module AUTHORS. And not by leveldb USERS then we can make more changes on it in an aggressive fashion.

Because honestly most of the initial module AUTHORS are you used to this idea being experimental. The module authors can then change the internals of their modules to match the new plugin / extension API and not break back compat on their surface api.

of course there are disadvantages to this. The module(db, { /* do stuff */ }) approach hides the fact that the module extends the database and intercepts puts. Which may be suprising.

ghost commented 11 years ago

yes. Private API and a Public API. Its a good way to give room to grow.

g

On 9 March 2013 23:45, Raynos notifications@github.com wrote:

Also if the plugin API is only used by module AUTHORS. And not by leveldb USERS then we can make more changes on it in an aggressive fashion.

Because honestly most of the initial module AUTHORS are you used to this idea being experimental. The module authors can then change the internals of their modules to match the new plugin / extension API and not break back compat on their surface api.

of course there are disadvantages to this. The module(db, { /* do stuff */ }) approach hides the fact that the module extends the database and intercepts puts. Which may be suprising.

— Reply to this email directly or view it on GitHubhttps://github.com/rvagg/node-levelup/issues/92#issuecomment-14672314 .

Contact details: +49 1573 693 8595 (germany) +46 73 364 67 96 (sweden) skype: gedw99