plexinc/papr

Proposal: No-op on empty arrays in `bulkWrite`

Closed this issue · 1 comments

ejmartin504 commented

In our usage of Papr, we often have code like the following:

const operations: BulkWriteOperation<Model>[] = [];

if (someCondition) {
  operations.push(updateOne: { { filter: {}, update: {}} });
}

If someCondition is falsey, and we call Model.bulkWrite(operations), we'll get a Mongo error:

Invalid BulkOperation, Batch cannot be empty

So, throughout our code we have have checks like this:

if (operations.length) {
   await Model.bulkWrite(operations);
}

Now, this is all well and good, but IMO it adds unnecessary additional cognitive overhead for developers, who must always remember to handle empty arrays. It's very easy to forget.

There may be very good reasons for the mongo driver to throw here rather than just silently do nothing, but I honestly can't think of any. Perhaps some digging through their source will reveal why. But in papr, we needn't follow their lead.

Proposed Solution

Handle empty arrays in papr's bulkWrite method, simply no-oping rather than throw-ing.

joshuat commented

I got bit by this earlier today - I'll put a PR addressing tomorrow.

We'll have to change the return type of Model.bulkWrite from Promise<BulkWriteResult> to include void, so even if the behavior change wouldn't require a breaking change - the type change would.

  • 👍1