pouchdb.d.ts icon indicating copy to clipboard operation
pouchdb.d.ts copied to clipboard

bulkDocs `docs` parameter shape?

Open AGBrown opened this issue 9 years ago • 3 comments

In the api documentation it states (emphasis added):

The docs argument is an array of documents.

and shows the code:

// Definition:
db.bulkDocs(docs, [options], [callback])
// Use:
db.bulkDocs([
  {title : 'Lisa Says', _id: 'doc1'},
  {title : 'Space Oddity', _id: 'doc2'}
]); //...

In test.basics.js for the 'Bulk docs' test it shows:

db.bulkDocs({
        docs: [
          { test: 'somestuff' },
          { test: 'another' }
        ]
      });//...

The difference is that in the api documentation the docs parameter is an array, but in the test it is an object, with a docs property that is an array.

Question

Which form should we add into the d.ts file?

  1. db.bulkDocs(docs: DocType[], options?: {}, ...
  2. db.bulkDocs(docs: { docs: DocType[] }, options?: {}, ...
  3. both of the above (which will get quite complicated)

AGBrown avatar Apr 30 '15 13:04 AGBrown

{docs: []} is the old syntax, since it's in the test file & doesn't print a warning it's probably not going to go away any time soon, but it's not recommended either since it's essentially just unnecessary typing. So option 3 would be possible, but 1 is enough for now.

marten-de-vries avatar May 03 '15 18:05 marten-de-vries

[14:03:36] <AndyBrown> anyone - what is the docs argument for bulkDocs? An array, or an object with a docs property that is an array - comments and help please: https://github.com/AGBrown/pouchdb.d.ts/issues/4 [14:16:26] AndyBrown: both are accepted [14:16:56] <AndyBrown> nolanlawson: thanks. Would you want both on the v3.4.0 d.ts, or just the one on the api docs? [14:17:03] and new_edits is allowed both on that obj and on the options obj [14:17:26] <AndyBrown> nolanlawson: dont think I've got to new_edits yet - haven't seen that [14:18:11] <AndyBrown> nolanlawson: but that might explain what I missed: in the 'Bulk docs' test does that mean the docs property is actually on the options object, and the docs argument has been omitted? [14:19:06] <AndyBrown> i.e. the "overload" is bulkDocs(options: { docs: DocType[] }, callback?: CallbackType) [14:19:57] <AndyBrown> (the overload in use in that test, that is) [14:20:27] <AndyBrown> and not bulkDocs(docs: { docs: DocType[]}, options?: {}, callback?: CallbackType) [14:32:45] AndyBrown: we pretty much accept all combinations ... [14:33:47] I believe docs is not on the options object, no [14:34:24] I.e. you can do bulkDocs({docs:...}, {new_edits:...}) [14:34:39] <AndyBrown> nolanlawson: I'll keep working through them, just pushed the latest code to feat branch - you can see the overloads in 3110b779, but from your last comment "...not on the options" it looks like I've got that overload wrong? (line 269 in that commit) ... [15:24:38] ... basically you can do all of the following: [15:24:47] db.bulkDocs(docsArray) [15:24:52] db.bulkDocs(docsArray, {new_edits: false}) [15:24:58] db.bulkDocs({docs: docsArray}, {new_edits: false}) [15:25:02] db.bulkDocs({docs: docsArray}) [15:25:07] db.bulkDocs({docs: docsArray, new_edits: false}) [15:25:27] db.bulkDocs({docs: docsArray}, {}) [15:25:32] db.bulkDocs(docsArray, {}) [15:25:37] I think those are all the possible combinations

AGBrown avatar May 03 '15 21:05 AGBrown

@marten-de-vries, that is helpful, thanks (relevant IRC added above).

I started working through the test.bulk_docs.js file and replicating in ts, which is proving to be an increasingly good way to make sure the d.ts: (a) lets me code everything that is reasonably expected from pouchdb's api; and (b) has the right balance between too simple and too complex a d.ts file. It is, however, a slow way to do this.

In 3110b779 I started to add in the overloads to go with option (3), and that has now evolved into 3732dcef. That lets me do things like the 'Testing new_edits=false in req body' test with helpful intellisense for the db.bulkDocs({ docs: docs, new_edits: false }, ... call

I'm still on the fence about doing option (3) as I now have a large number of overloads and I'm not sure the overload intellisense is working correctly as a result. However at least, first and foremost, it is possible to write all the required bulkDocs calls I have encountered in the tests until now.

(although that test has been temporarily removed again as I haven't done the correct get overloads yet).

AGBrown avatar May 03 '15 21:05 AGBrown