Incompatibility with Express 5

[pterrien]

Hi,

I faced a problem, when trying to integrate this lib with Express 5, and I thought this issue could be of some interest for you guys.
Although Express 5 is still alpha, you might want to consider it, as this is definitely an upcoming problem...

Here are my investigations, and 2 proposed workarounds / solutions.

Description of the problem

Starting from Express 5, req.query (corresponding to the URL query string) is now read-only (a getter, technically): see this link for more info.

Regarding pouchdb-express-router, the impact is that, basically, the following Express middleware in index.js (line 15) now becomes useless:

  app.use(function (req, res, next) {
    for (var prop in req.query) {
      try {
        req.query[prop] = JSON.parse(req.query[prop]);
      } catch (e) {}
    }
    next();
  });

=> req.query won't get modified anymore, whatever you do here...

The issue is linked to the fact that pouchdb-express-router passes the URL query parameters directly to PouchDB functions (e.g. db.get(...)), and those won't get transformed anymore.
It still works in most of the requests, when params are strings, but it fails when an array is passed, like on document, with the open_revs param.

Solutions / Workarounds

Solution 1: tweak the Express query parser

Starting from Express 5, the only way to modify req.query is from the query parser, called before any of the routers (this link for more info).
Instead of using the standard query parser, we can define our own, still based on qs, just to add an additional JSON parse step:

  const qs = require('qs')
  app.set('query parser', function (str) {
    const query = qs.parse(str) // call the standard parser
    Object.keys(query).map(key => {
      try {
        query[key] = JSON.parse(query[key])
      } catch (e) {} // silently ignore
    })
    return query
  })

Important note: this is linked to Express app, not router, which makes it out of this library's scope, on a strict point of view.
I don't find this approach neither efficient nor elegant, but it works, and could be an acceptable workaround for some people I guess.

Pros:

• nothing to modify in this library, except maybe some guidance in the main README.md file, to indicate people how to integrate this router.

Cons:

• we mainly break the segregation of concerns, by modifying the app behavior, because of an issue with this router:
  • the query parser should be used to parse HTTP queries, not to implement some kind of application-related logic
  • what is related to the router should stay at the router level

Solution 2: do no use req.query anymore, put PouchDB stuff in res.locals instead

The Express-way of storing user-defined data is to use res.locals.
So it's quite simple: we could modify the middleware, to store JSON parsed values in res.locals.query instead of req.query (or res.locals.pouchdb.query, or anything you'd prefer).
And then modify all the route functions, to get data from res.locals.query instead of req.query.

The middleware could be as simple as:

  app.use(function (req, res, next) {
    if (!res.locals.query) {
      res.locals.query = {}
    }
    for (var prop in req.query) {
      try {
        res.locals.query[prop] = JSON.parse(req.query[prop])
      } catch (e) {
      	res.locals.query[prop] = res.query[prop] // copy value with no parsing
      }
    }
    next()
  })

Pros:

• cope with Express best practices
• make this library easier to integrate: it works, with no configuration or tweaks

Cons:

• some adjustments on source code, but they should be quite limited after all

Conclusion

I hope you will find this issue interesting, and maybe it will bring some help to people struggling with Express 5 the way I did.

As you may have anticipated, I would certainly go with solution 2, but I'll let you decide which one you prefer, of course.
If needed, and depending on your decision, I can take some time to make a proper PR if you'd like.

Many thanks for your work on this great library, I think it perfectly matches an important need: a lightweight and agnostic Express router for PouchDB, we need it!.
Not a full-stack app or server, no fancy UI... just a router.
Let's keep simple things... simple! :)

Hope it will help,
Keep up the good work guys!

Cheers,

Pascal.

[pterrien]

also created a ticket for pouchdb/pouchdb-server, which should be impacted the same way.

[pterrien]

the issue has other impacts: db changes are not propagated to clients, when using solution 1...
So I modified the lib on my side, basically doing a simplistic search & replace:

• req.query-> res.locals.pouchdb.query
• req.db -> res.locals.pouchdb.db
  and adding the middleware above (except I now group everything in a res.locals.pouchdb object)

=> it seems to work fine, but I'm afraid I'm not experienced enough w/ PouchDB to perform proper testing

I'll be happy to share if anyone is interested!

[alxndrsn]

5 years later, the current version of express is still 4.x. Closing in line with pubkey/rxdb#981 (comment):

I will close this issue because express5 is still in alpha which means that neither rxdb nor pouchdb-express should do a premature fix which maybe will not work when express5 is released.