pouchdb / pouchdb

:kangaroo: - PouchDB is a pocket-sized database.
https://pouchdb.com/
Apache License 2.0
16.9k stars 1.47k forks source link

Document db.request() #3525

Closed nolanlawson closed 8 years ago

nolanlawson commented 9 years ago

It's come up a lot (e.g. https://github.com/pouchdb/pouchdb/pull/3522) and it's really useful, so it should be in the documentation.

marten-de-vries commented 9 years ago

The scenario for which it comes up is in all cases exposing extra CouchDB functionality not available on the http adapter, though. It might be worth instead adding the following http methods:

It would be pretty trivial. Local implementations could be handled by plug-ins, or e.g. in the case of purge, be implemented later. The upside would be that it makes PouchDB a lot more usable as a http lib, the downside is size.

NickColley commented 9 years ago

The API at the moment is local-http, so having so many http specific functions might be confusing? Shouldn't this be handled only by plugins to avoid API bloat?

nolanlawson commented 9 years ago

request() is already in heavy use by various plugins (pouchdb-find, pouchdb-authentication, etc.), so given that plugin authors are expected to be able to write their stuff with no more knowledge than what's in the API, I still think it's reasonable to document.

marten-de-vries commented 9 years ago

@nickcolley It doesn't have to be confusing if it returns a nice error on a local db. That again means adding to the bundle size, but it does make PouchDB a nicer replacement for other CouchDB libs.

@nolanlawson request() is an internal convenience function, I don't expect that in the public api of a database myself. I would prefer for #3524 to happen instead: that way plug-ins and pouchdb can both just require the implementation and it doesn't have to become part of the public API. (I know there is still a tiny difference in that the first doesn't automatically add the db name, but the newly released package could provide a convenience function for that.)

I don't use request() in my own plug-ins btw, I just use plain xhr and drop in the xhr2 package for node support.

marten-de-vries commented 9 years ago

3529 is another (apparently independent) case where a getSecurity()/putSecurity() on the http adapter would've made stuff easier. My current preferences would be:

nolanlawson commented 9 years ago

Fair enough, in that case it may just be two of my plugins (find and authentication) that use db.request. I'm fine with having it externally require-able.

daleharvey commented 9 years ago

So to mirror what I mentioned in the other thread, right now our exposed api are things that can be implemented across the various adapters, its why allDbs got cut (it may be why temp views and compaction get cut)

db.request is the one API I would consider breaking the rules for, its just a really nice API for users especially when they may have already setup a fairly complicated connection to just be able to fire requests without worrying about setting up the host / connection again. I think it lets people easily access the functionality we dont support, its clear that it will only work against a remote database and we dont have to worry about version / implementation issues for things like security while still being able to use plain pouchdb core as a very usable couchdb http client

I am +0.9 on just documenting db.request

(+1 on having browser-request externally requireable, that will happen anyway)

daleharvey commented 8 years ago

So yeh I have changed my thinking on this especially now that we have browser-request as an externally requirable module, I think we should be telling any users who need to make arbitrary requests to a couchdb server to use that (or any other ajax lib) and we should keep our API as consistent across adapters as we possibly can. So I would like to close this out, @nolanlawson any objections?

daleharvey commented 8 years ago

Objection can be raised in the form of reopening or PR :)

cboden commented 8 years ago

I'm working with CouchDB and using PouchDB in Node as my agent. I'm looking for documentation on .request() but can't find it in the API page on the website. Just wondering why this issue to document the request method was closed?

daleharvey commented 8 years ago

PouchDB aims to provide a backend agnostic api, all of our core methods should work transparently no matter which backend you are using, indexeddb / websql / http. db.request is not agnostic however, its very specific to http. It was written as an internal convenience function that the other methods call and exposing it was mostly a mistake and limitation of how we built pouchdb.

Having a fully fledged CouchDB client is a full project on its own and would conflict with the aims of PouchDB to provide a minimal in browser storage, db.request provides a mostly inadequate http client that would be better served by either dealing with the ajax requests yourself (via xhr, request, jquery etc) or a dedicated couchdb http client (which pouchdb is not)

nolanlawson commented 8 years ago

Instead of db.request() there is also db._ajax() now, although it's obviously internal and private.

nolanlawson commented 8 years ago

require('pouchdb-ajax') is the new recommended solution moving forward