pouchdb / upsert

PouchDB plugin for upsert() and putIfNotExists() functions
Apache License 2.0
149 stars 25 forks source link

id not included in res. #30

Closed jordancardwell closed 7 years ago

jordancardwell commented 7 years ago

The id isn't included in the res object.. any reason for that?

I assumed it'd be in keeping with the response you get from the standard pouchdb PUT ...something like:

{
  "ok": true,
  "id": "mydoc",
  "rev": "1-A6157A5EA545C99B00FF904EEF05FD9F"
}

but instead we get

{
  "updated": true,
  "rev": "..."
}

and I have to merge in id/ok to allow code that used to rely on the PUT to handle the upsert response

return {
  ok: res.updated,
  id: doc._id,
 ...
}

Thanks for the lib, and sorry if this was addressed in a previous issue.. going to continue checking and close this if I find my answer there :+1:

nolanlawson commented 7 years ago

Hm I think the reason I didn't add it is I didn't see any cases where I absolutely needed the id... e.g. you are passing in the id yourself, so presumably you know what it is. :smiley: although maybe for some code patterns it could be awkward, I agree.

jordancardwell commented 7 years ago

Thanks for the prompt response.

I agree it's certainly not absolutely needed, but I would argue that folks will often use this in place of standard pouchdb PUT/POST functions, which return the id in their responses, even in the case of PUT, where the id is available/known before hand as well. So one might argue that an implicit convention or precedent was established to return the id as a convenience, even though you could grab it from within the context of the callback or .then.

Would you be strongly opposed to adding the id to the returned object? (I wouldn't mind adding it and opening a PR for discussion, if you're open to it)

I think adding ok would be reasonable as well.. though I'm not certain whether it would always be equal to updated and I would just .catch any errors that upsert might throw, so I don't feel very strongly either way. Though the ease of adding and potential benefit to others would outweigh any negative impacts I could contrive.

Just let me know how you feel about the PR for adding id, and, if you're okay with it, whether you'd like to throw it in yourself or have me open a PR.

Thanks!

nolanlawson commented 7 years ago

fixed in #31