Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: monorepo package names and plugin format #5162

Closed
nolanlawson opened this issue May 15, 2016 · 23 comments
Closed

RFC: monorepo package names and plugin format #5162

nolanlawson opened this issue May 15, 2016 · 23 comments

Comments

@nolanlawson
Copy link
Member

nolanlawson commented May 15, 2016

(Previous discussion: #5137 #5128)

TLDR: I have three questions for the community:

  1. @pouchdb/ or pouchdb- prefixes for package names?
  2. How to name plugins vs adapters vs presets?
  3. I propose a new, backwards-compatible plugin API. Is it okay?

Private modules vs prefixes

When we publish the monorepo packages, we'll have a choice between the prefix style used by Babel, React, and Ember:

  • pouchdb-foo
  • pouchdb-bar

And the "private module" pattern, which is a bit new, but is being used by Angular 2 and Hoodie at least:

  • @pouchdb/foo
  • @pouchdb/bar

Each one has benefits and detriments. There is some discussion in this Twitter thread. So question one is whether we go with the private module style or the prefix style.

(I've already registered a npm user named pouchdb, which we can apparently upgrade to an organization once organizations are free for open-source.)

Package naming conventions

Second question is what to name the modules. I propose the following:

  • pouchdb
  • @pouchdb/core
  • @pouchdb/plugin-mapreduce
  • @pouchdb/plugin-replication
  • @pouchdb/plugin-adapter-indexeddb
  • @pouchdb/plugin-adapter-websql
  • @pouchdb/plugin-adapter-node-websql
  • @pouchdb/plugin-adapter-leveldb
  • @pouchdb/plugin-adapter-memory
  • @pouchdb/plugin-adapter-localstorage
  • @pouchdb/plugin-adapter-fruitdown
  • @pouchdb/preset-browser
  • @pouchdb/preset-node
  • @pouchdb/ajax
  • @pouchdb/generate-replication-id
  • @pouchdb/checkpointer
  • @pouchdb/promise

Notice the plugin- prefix for plugins, plugin-adapter prefix for adapters (which are also plugins), and the preset- prefix for the "Node version" of PouchDB and the "browser version" of PouchDB. What we used to call "extras" have no prefix. (But maybe they should?)

There are also an odds-and-ends modules like @pouchdb/utils and @pouchdb/errors that are mostly grab-bags of different utilities used across the whole codebase. I propose marking these as 0.0.x so we don't need to make any claims for SemVer. Also I think it's pointless to document them, except maybe with a README.

New plugin API

In order to enable modules like replication and the adapters to be used as "plugins," I propose adding one new feature to the PouchDB.plugin() API: in addition to passing in an object, you can pass in a function that is given the PouchDB object, and therefore you can do what you want with it. This is useful for adapters:

// @pouchdb/plugin-adapter-indexeddb
export default function (PouchDB) {
  PouchDB.adapter('idb', IdbPouchAdapter, true);
}
// user code
import IdbPouch from '@pouchdb/plugin-adapter-indexeddb'

PouchDB.plugin(IdbPouch);

And e.g. replication:

// @pouchdb/plugin-replication
export default function (PouchDB) {
  PouchDB.sync = sync;
  PouchDB.replicate = replicate;
}

The end result is that we have a nice clean plugin API that hides internal details like the PouchDB.adapter API. It also allows things like the http adapter adding both 'http' and 'https', the replication plugin adding PouchDB.replicate and PouchDB.sync, and third-party plugins that might bundle multiple plugins together.

Here's a full example of what @pouchdb/preset-browser would look like:

import PouchDB from '@pouchdb/core';

import IDBPouch from '@pouchdb/plugin-adapter-indexeddb';
import WebSqlPouch from '@pouchdb/plugin-adapter-websql';
import HttpPouch from '@pouchdb/plugin-adapter-http';
import mapreduce from '@pouchdb/plugin-mapreduce';
import replication from '@pouchdb/plugin-replication';

PouchDB.plugin(IDBPouch);
PouchDB.plugin(WebSqlPouch);
PouchDB.plugin(HttpPouch);
PouchDB.plugin(mapreduce);
PouchDB.plugin(replication);

export default PouchDB;

Thanks for your feedback! :) Everything is working pretty well technically right now; I just want to make sure we can reach a consensus on naming convention, style, etc.

@geppy
Copy link

geppy commented May 15, 2016

  1. I'd definitely be in favor of scoped modules.
  2. I like your convention of prefixing all plugin module names with plugin-. Though I expected adapters not to have that prefix, it seems like it makes more sense to avoid special casing that.
  3. The plugin registration API doesn't seem bad. Is there a reason not to have something like addPlugins(...plugins)? I don't know what I'd call the registration method, but I'd prefer an API that used a rest parameter over one with a purely unary API.

@NickColley
Copy link
Contributor

NickColley commented May 15, 2016

This is really exciting seeing it all laid out, definitely going to be a great addition.

  1. Scoped seems like a good way forward generally.
  2. For third party authors of plugins what would it look like? I'm imagining if you're including a 'core' plugin and a 'third party' plugin it'd look like the following:
import PouchDB from '@pouchdb/core';

import IDBPouch from '@pouchdb/plugin-adapter-indexeddb';
import FooBarPouch from 'pouchdb-plugin-foobar';

PouchDB.plugin(IDBPouch);
PouchDB.plugin(FooBarPouch);

export default PouchDB;

So maybe a little asymmetry but I don't think it's an issue?

3 . I like the simplicity for the plugin API, I'd have thought it'd enable lazy-loading down the line too?

@nolanlawson
Copy link
Member Author

@geppy How about if .plugin() could be chained? This would be backwards compatible:

PouchDB.plugin(foo)
  .plugin(bar)
  .plugin(baz);

@geppy
Copy link

geppy commented May 15, 2016

@nolanlawson I think that'd be great.

@nolanlawson
Copy link
Member Author

@NickColley The asymmetry is desired. @pouchdb/foo communicates that it's a first-party plugin because only we have the credentials to publish to @pouchdb. pouchdb-foo communicates that it's third-party.

As for lazy-loading, yes, you should be able to trigger-load a plugin.

Also I just realized the plugin API actually already does support chaining; we just didn't document it. 😅

@nolanlawson
Copy link
Member Author

Counterargument against private modules: it sure feels more natural to npm install pouchdb-browser than npm install @pouchdb/preset-browser. Are we too far ahead of the curve here? Or maybe should we just drop the preset- part?

@nolanlawson
Copy link
Member Author

In fact the more I think about it, the more I can't get over how ugly and hard-to-type those private modules are. Just try saying them aloud. 😛

It's worth pointing out that Lerna is working on adding the ability to manage all npm owners across multiple packages, which would also limit the benefit of having "one" account - which at this point is also still a user rather than an organization, meaning you'll have to change your npm credentials when you publish.

Hm, that kinda stinks. I feel like I'm leaning toward prefixes rather than private modules now.

@colinskow
Copy link
Member

To me shorter names are better. Can you cut the plugin- from the adapters? I don't think it adds much value.

@colinskow
Copy link
Member

And add a preset-http with the HTTP adapter only.

@nolanlawson
Copy link
Member Author

nolanlawson commented May 15, 2016

And add a preset-http with the HTTP adapter only.

I was planning on making this a community thing. There are many combinations I can think of (http with mapreduce, http without mapreduce, websql only, etc.), so I think it's best to take the stand not to support them at all. It will be like 4 lines of code to write your own. :)

The reason pouchdb-preset-browser and pouchdb-preset-node are in there is because they are shipped by pouchdb. (pouchdb just depends on those two.)

Can you cut the plugin- from the adapters?

The adapters are also plugins; you load them by doing PouchDB.plugin(). I think it's best to have consistency here, because not every module exposes a plugin API.

@colinskow
Copy link
Member

HTTP-only is a very common use case. I've got several apps that only fetch data from a remote database. For the amount we can reduce the file size, I think this is well worth having official support.

I'm for 3 official presets:

  1. browser
  2. node
  3. http-only

@colinskow
Copy link
Member

For ease of loading plugins, how about:
PouchDB.plugin( [foo, bar, baz] );

@nolanlawson
Copy link
Member Author

Hm, how about the name pouchdb-http for the http-only one? I agree it's a common use case; maybe it does make sense to support.

I feel like plugin( [foo, bar, baz] ) is a bit unintuitive (plugin singular vs plugins plural). Is PouchDB.plugin(foo).plugin(bar).plugin(baz) really too much typing? Here's an example of pouchdb-browser as I've currently written it:

PouchDB.plugin(IDBPouch)
  .plugin(WebSqlPouch)
  .plugin(HttpPouch)
  .plugin(mapreduce)
  .plugin(replication);

@daleharvey
Copy link
Member

Can you cut the plugin- from the adapters?

The adapters are also plugins; you load them by doing PouchDB.plugin(). I think it's best to
have consistency here, because not every module exposes a plugin API.

I agree with this, its a fairly minor details but if every adapter is a plugin I dont think we need the extra prefix.

I propose a new, backwards-compatible plugin API. Is it okay?

If its backwards compatible and allows adapters to be installed via it, it looks great to me.

@pouchdb/ or pouchdb- prefixes for package names?

So I dont have much of a preference vs either, but I do think we should be making a distinction between packages designed specifically for user consumption (pouchdb, pouchdb-browser, pouchdb-http) etc than ones that are pretty much internal pouchdb/promise / pouchdb/plugin-adapter-leveldb, maybe scoped packages for internal and normal packages for external? if the hassle of scoped packages are too much then happy not doing that, but I would like to make sure we are being considerate of what we document / tell end users to use, I wouldnt like to be in a situation I have seen in other libraries where using it requires understand a variety of modules / how they fit together and which ones they should be using. pouchdb/plugin-adapter-memory is a nice handy module for those people who really need access to it, but it shouldnt be in a position to confuse people who want to using standard pouchdb in the browser.

@nolanlawson
Copy link
Member Author

nolanlawson commented May 16, 2016

OK, I'm fine with cutting the plugin- prefix for adapters. It is indeed a bit long.

For the "internal" modules, I'd rather not have a mix of scoped and non-scoped, so maybe we can just not document them and set the SemVer to 0.0.x? We've already been effectively publishing lots of these things (pouchdb-collections, pouchdb-ajax, etc.) and I haven't seen much user confusion, because nobody finds them anyway. 😀 I imagine it would be the same for our new internal modules like pouchdb-utils and pouchdb-errors (and heck, even pouchdb-promise, which I don't see much need to document).

BTW I've opened up a vote for scoped vs prefixed, so let's move that discussion over here: #5165

@marten-de-vries
Copy link
Member

Making it possible for plugins to act immediately on the PouchDB object is a very nice extension, in the past some 3rd-party plugins have struggled with that too (e.g. pouchdb-all-dbs, pouchdb-seamless-auth).

Chaining .plugin() is fine with me, array syntax would work but I don't really like it. Registering tons of plugins is rare so it seems like too much.

As for package naming, no strong opinions there.

@nolanlawson
Copy link
Member Author

nolanlawson commented May 16, 2016

'Nother question... do we even want the plugin- or preset- prefixes at all? I'm kind of thinking that there are so few of them, that there's no point. adapter- makes sense, but maybe not plugin. Here's how it would look (with prefixed packages):

pouchdb
pouchdb-adapter-fruitdown
pouchdb-adapter-http
pouchdb-adapter-indexeddb
pouchdb-adapter-leveldb
pouchdb-adapter-leveldb-core
pouchdb-adapter-localstorage
pouchdb-adapter-memory
pouchdb-adapter-node-websql
pouchdb-adapter-websql
pouchdb-adapter-websql-core
pouchdb-ajax
pouchdb-browser
pouchdb-checkpointer
pouchdb-core
pouchdb-errors
pouchdb-generate-replication-id
pouchdb-http
pouchdb-mapreduce
pouchdb-mapreduce-utils
pouchdb-node
pouchdb-promise
pouchdb-replication
pouchdb-utils

I kind of like just keeping it simple. The READMEs can explain what exactly the modules do.

@marten-de-vries
Copy link
Member

I like it, this would also mean less package names would have to be deprecated, which could prevent some confusion. (e.g. no pouchdb-mapreduce -> pouchdb-plugin-mapreduce). Furthermore, since all would use the same API to 'register' anyway (.plugin() if I understand your new API proposal correctly), it seems from the user perspective there actually isn't that much difference.

That said, names do matter to keep track of things. 'adapter' definitely makes things clearer, 'preset-' might as well (uncertain there).

@nolanlawson
Copy link
Member Author

Not everything would register via plugin(). E.g. the extras (pouchdb-ajax, pouchdb-checkpointer, pouchb-promise) would just export the "thing" they're supposed to export. But again, this can be explained in READMEs.

The only reason I'm unsure about preset is that I really like the idea of typing:

  • npm install pouchdb-browser
  • npm install pouchdb-node
  • npm install pouchdb-http

@daleharvey
Copy link
Member

I prefer pouchdb-browser to pouchdb-preset-browser

As for versioning, we do the -prerelease dance because not all of our users are coming via npm, we have / had nightlies and I think we would like to preserve the ability to produce nightly versions while maintaining that they are obviously not release builds, it helps in debugging users issues a bit. I am not tied to any specific way of doing this, but I would like to be able to produce a build, give it to someone and for them to not later be confused that they are using a released distribution

@broerse
Copy link
Contributor

broerse commented May 18, 2016

I am for the prefix style. Will vote. Also perhaps adopt a ember-addon like keyword in package.json

"keywords": [
  "ember-addon",
  ...

pouch-plugin ?
Or call plugins, adapters, presets addons ?

It will make it possible to find them after we have created 2000 addons just like https://www.emberaddons.com/ ;-)

@nolanlawson
Copy link
Member Author

@broerse That's a good idea; we really should have done that years ago before people started writing plugins though. 😅 I can do "pouchdb-plugin" for the first-party plugins though.

@daleharvey No worries, it'll be called pouchdb-browser and pouchdb-node. Also I managed to keep our -prerelease system; see #5178

@nolanlawson
Copy link
Member Author

Closing because this seems more-or-less decided, please move discussion over to #5178.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants