Deprecate pouchdb-req-http-query in favor of request / request-shim

[loic]

pouchdb and pouchdb-req-http-query (and all plugins that depend on it) use different http libraries, which means they don't share options and on node they don't share cookies. As a result on node pouchdb-auth's logIn() methods don't work transparently between local and remote databases.

If we want to keep the pouchdb-req-http-query plugin around, one option could be to keep its public API but swap xmlhttprequest-cookie with request, that would at least solve the cookie jar problem.

@marten-de-vries wdyt?

[loic]

Relevant discussions I could find on the topic: https://github.com/pouchdb/pouchdb/issues/6944 https://github.com/pouchdb/pouchdb/issues/5322 https://github.com/pouchdb/pouchdb/issues/3525

[marten-de-vries]

The reason pouchdb-req-http-query uses CouchDB request objects as its API is that that is the same API as pouchdb-show, pouchdb-list, etc. use as input, and pouchdb-rewrite uses as output. Implementing it that way was convenient for usage in the browser & implementing HTTP support. As such, I think it would be hard to use another HTTP query API. It would mean function API changes everywhere.

That leaves rewriting pouchdb-req-http-query in terms of a non-XMLHttpRequest api, or changing the XHR shim (https://github.com/rse/node-xmlhttprequest-cookie) in node to use request's cookie jar. That's fine with me. Currently, it works in both the browser and node and also does not require a huge shim in the browser. Ideally, a new implementation would have the same properties, which is possible but challenging.

On second thought, the easiest fix might actually be to just use the current code when running in the browser, and then to write another path from scratch for usage in node based on request. It duplicates code, but the XHR code has never needed maintenance while the shim has been finetuned multiple times by now IIRC. It would also simplify the code: this block could go, as could this check.

[marten-de-vries]

Looking at the code: I think it actually worked one time due to this line: https://github.com/newlogic42/pouchdb-server/blob/fb1fea5f8a34e21a61fe2c3c5e72857ec74fe33e/packages/node_modules/pouchdb-req-http-query/lib/index.js#L76

Getting that line to work again somehow might also be a solution.

[loic]

Thanks @marten-de-vries for the great feedback as usual.

changing the XHR shim (https://github.com/rse/node-xmlhttprequest-cookie) in node to use request's cookie jar

I attempted that already, but request global cookie jar is out of reach (didn't find any accessors) and since https://github.com/pouchdb/pouchdb/commit/17db84f54d3bf432a60faeb632ce383e8464d3b1 that's what pouchdb enables.

the easiest fix might actually be to just use the current code when running in the browser, and then to write another path from scratch for usage in node based on request.

Seems like a good solution! It seems consistent with the approach taken in https://github.com/pouchdb/pouchdb/pull/6988/files (except that it switches to fetch API instead of XHR for browser).

Looking at the code: I think it actually worked one time due to this line: https://github.com/newlogic42/pouchdb-server/blob/fb1fea5f8a34e21a61fe2c3c5e72857ec74fe33e/packages/node_modules/pouchdb-req-http-query/lib/index.js#L76

I'm gonna investigate, if it stopped working due to a minor breaking change that could indeed be the easiest way to go.

[loic]

I'm struggling a bit. Switching to request is not enough, because pouchdb embeds its own request, so the global cookie jars aren't the same.

And since it seems pouchdb doesn't know anything about headers anymore as it now delegates all HTTP handling to request which doesn't lend itself for introspecting, I can't restore the old db.getHeaders() either...

I wish pouchdb would at least expose the HTTP client for other plugins to use...

[marten-de-vries]

If you use require('pouchdb-ajax'), you get the same version as PouchDB I think. That gives you a wrapper around request, though. Perhaps changing that package a bit to also expose require('pouchdb-ajax/request') or something similar would work?

[loic]

require('pouchdb-ajax') is too ~magical~ specific for me to confidently handle the response in a way that remains compatible with the existing output of pouchdb-req-http-query.

We probably don't have much other choice than exposing request. I wonder though why it's repackaged? Maybe it's a side-effect of the ES6 transpilation step. Sadly we are going to be blocked again until there is another release of pouchdb.

[marten-de-vries]

It's a side effect of the transpilation step indeed. This line translates into a local variable.

We really need an alternative to having to wait for a release. This is going to keep coming up for any serious pouchdb-server development. Not sure what the best way would be, though.

[loic]

I think even the recommended way of require('pouchdb-ajax') wouldn't work for the same reason, pouchdb uses its own transpiled pouchdb-ajax...

[marten-de-vries]

Right, so that leaves us with exposing (e.g.) PouchDB._requestPleaseDontUseThis. Interesting how the downsides of rollup & monorepo come cropping up with this issue.

[loic]

We really need an alternative to having to wait for a release. This is going to keep coming up for any serious pouchdb-server development. Not sure what the best way would be, though.

I guess that's the downside of having 2 monorepos that depend on each other through npm. One option would be to have an officially blessed Github fork of pouchdb that pouchdb-server can use between releases so at least development doesn't stall (what I did earlier with my dist-pouchdb GH repo).

[marten-de-vries]

I think it's better to use an npm package than a git repo with a manually created PouchDB build. If we divert too much from the release process of the 'real' pouchdb, we risk introducing incompatibilities between the two.

Perhaps nicest would be to release a beta version of the pouchdb npm package. I think PouchDB's release process actually supports that already, but I never looked into it in detail as I don't have the required npm rights.