new @hoodie/server #500
new @hoodie/server #500
Conversation
BREAKING CHANGE: The `@hoodie/server` package was deprecated as its logic was merged into `hoodie`. But after further discussion, we decided to move the hapi logic out into its own module and reuse the `@hoodie/server` package for it. See hoodiehq/hoodie#534
gr2m
force-pushed
the
hoodie/534/new-hoodie-server
branch
5 times, most recently
from
4512484
to
3d35315
Compare
gr2m
force-pushed
the
hoodie/534/new-hoodie-server
branch
7 times, most recently
from
c2ac166
to
1415acd
Compare
| function getConfig (server, config, callback) { | ||
| defaultsDeep(config, getDefaults()) | ||
|
|
||
| var adapter = config.PouchDB.preferredAdapters[0] |
There was a problem hiding this comment.
Seems a bit of a significant change, and I think it conflicts with this line in the readme at present: https://github.com/hoodiehq/hoodie-server/pull/500/files#diff-04c6e90faac2675aa89e2176d2eec7d8R59 - specifically: "store.couchdb, store.PouchDB are set based on db option above".
Seems there's no db option anymore, and it requires the user to setup their own PouchDB instance prior to passing options @hoodie/server?
There was a problem hiding this comment.
Providing the user does what this file did (https://github.com/hoodiehq/hoodie/blob/82897fd9da994e88f4a0aae30e7c6de02f04dad1/server/config/db/factory.js), then it works? (I'm about to test)
There was a problem hiding this comment.
The idea is that @hoodie/server does not have pouchdb as dependency, hence the .PouchDB option. Does that make sense?
Note that you’ll still be able to require hoodie as package and it will continue to return a hapi plugin, but if you only need APIs and need more control, you can now directly require the "lower level" @hoodie/server.
it conflicts with this line in the readme at present: https://github.com/hoodiehq/hoodie-server/pull/500/files#diff-04c6e90faac2675aa89e2176d2eec7d8R59
Good catch, I’ll fix it
Providing the user does what this file did (https://github.com/hoodiehq/hoodie/blob/82897fd9da994e88f4a0aae30e7c6de02f04dad1/server/config/db/factory.js), then it works? (I'm about to test)
We actually don’t need this factory workaround any longer, because PouchDB’s prefix option now applies to the http adapter as well, so we can get rid of it :)
|
I think my only question is around the DB creation... Is there a reason we're not just taking what's currently in |
|
|
||
| function accountConfig (state, callback) { | ||
| var usersDb = new state.config.PouchDB(state.db.authenticationDb) | ||
| usersDb.installUsersBehavior() |
There was a problem hiding this comment.
Are we missing pouchdb-users?
There was a problem hiding this comment.
Hmm I remember struggling myself to decide what the right place is for pouchdb-users. I think as we pass in the PouchDB constructor, I feel like we should apply all plugins beforehand. On the other side we can apply it within @hoodie/server, too ... maybe that’ll make things simpler? I’ll give it another thought, thanks!
There was a problem hiding this comment.
I think that while the logic that depends on this plugin exists in @hoodie/server, we should probably have pouchdb-users as a dependency - otherwise there'll be quite a spread of code between hoodie and hoodie/server?
Makes sense that the server relies on a PouchDB constructor, too - means they have the control over the instance etc.
There was a problem hiding this comment.
okay, thanks, I’ll give this a go!
There was a problem hiding this comment.
Have been thinking about this a bit more...
What happens if the plugin is already registered? Giving the user power to define their own PouchDB constructor also means they're likely using their own plugins, and wouldn't be surprised if they need this plugin somewhere down the line.
Binding it here may mean that users have to wait until @hoodie/server has been registered to take advantage of the pouchdb-users plugin. So I suppose either way, we need to document that it's included (therefore don't try to register it), or that it's not - and therefore the user needs to register it.
|
So I got this working by creating the following PouchDB constructor: var PouchDB = require('pouchdb').defaults({
prefix: 'my couch db url here'
}).plugin(require('pouchdb-users'))So seems we're missing |
|
My PouchDB constructor is connecting to a couchdb instance, and it appears I'm getting a bunch of folders in the root of my project... Feels like this came up a few weeks ago elsewhere? |
|
I've run into his myself and am fixing it already, work in progress |
gr2m
force-pushed
the
hoodie/534/new-hoodie-server
branch
from
63de65b
to
8bb764d
Compare
|
Been running this in place of LGTM! |
|
LGTM \o/ |
|
w00p w00p, released as v20.0.0 |
in preparation of hoodiehq/hoodie#534