Fix remove & other promise returning functions

[rafamel]

• First commit addresses RxDocument.remove() exclusively.
• Second commit addresses all promise returning functions, either documented as such or identified as far as I've been able to, so they always reject rather than sync throw, either explicitly (throw called), via property access, or else.
  • There's the edge case of __ensureEqual on rx-query.js, which is documented as returning either a promise or a boolean, so it hasn't been modified.
  • Some functions on util.js like nextTick are not susceptible of erroring out, so they were not modified.

[pubkey]

This change increased the minfied size by 1kB.
I do not think we should use the async-keyword on every function that returns a promise.
For example countAllUndeleted() will be transpiled to

/**
 * @return {Promise}
 */
_proto2.countAllUndeleted =
/*#__PURE__*/
function () {
  var _countAllUndeleted = _asyncToGenerator(
  /*#__PURE__*/
  _regeneratorRuntime.mark(function _callee3() {
    return _regeneratorRuntime.wrap(function _callee3$(_context4) {
      while (1) {
        switch (_context4.prev = _context4.next) {
          case 0:
            return _context4.abrupt("return", PouchDB.countAllUndeleted(this.pouchdb));

          case 1:
          case "end":
            return _context4.stop();
        }
      }
    }, _callee3, this);
  }));

  return function countAllUndeleted() {
    return _countAllUndeleted.apply(this, arguments);
  };
}();

And before it was:

_proto2.countAllUndeleted = function countAllUndeleted() {
  return PouchDB.countAllUndeleted(this.pouchdb);
};

[rafamel]

Sorry for the delay @pubkey , I've been pretty caught up this past week. I would personally make that trade-off, as I'd have the ensurance that async returning functions will not throw and interrupt execution without having to think about anything else than declaring them as async, and hopefully we'll be at the point of not needing transpilation for async/await soon enough. But I can definitely see your point. An alternative would be to either only do this for public functions and methods, or do try/catch and return a promise rejection for them. What would be your preference?

[pubkey]

I think I do not fully understand the benefits of the change.
For example the function getBatch() only uses the pouchdb's getBatch and returns the modified result. This means that whatever you do, you get a promise back when the function is called. When we add async to the function, does this change anything at all? (implied that the pouchdb.getBatch is called the right way)

[rafamel]

Hi @pubkey , the only benefit here would be ease of mind. Using async guarantees us that for promise returning functions, we'll have promise rejections. Otherwise, we'd need to think on a case-by-case basis on whether there could be any property access or otherwise unforeseen error that could throw and interrupt execution, particularly as for promise returning functions error handling might be done via catch on the user part, or even by ourselves in other parts of the application/library. Always using async, regardless of context, spares us of having to think about it, and is also a syntactically sweeter way of doing this in opposition to a try wrapping the whole function and explicitly rejecting. For that reason only, as a general rule, I tend to declare every promise returning function as async; doing so spares me from a case-by-case analysis so I can focus on implementation, and prevents that if the logic of the function changes over time, I forget to either try/catch or declare as async, as it is always already declared; it also prevents that edge cases I didn't take into account interrupt execution. But again, this is just the way I personally balance that trade-off.

[pubkey]

Closed, see here