Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bulkDocs docs parameter shape? #4

Open
AGBrown opened this issue Apr 30, 2015 · 3 comments
Open

bulkDocs docs parameter shape? #4

AGBrown opened this issue Apr 30, 2015 · 3 comments

Comments

@AGBrown
Copy link
Owner

AGBrown commented Apr 30, 2015

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)
@marten-de-vries
Copy link

{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.

@AGBrown
Copy link
Owner Author

AGBrown commented May 3, 2015

[14:03:36] 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: #4
[14:16:26] AndyBrown: both are accepted
[14:16:56] 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] nolanlawson: dont think I've got to new_edits yet - haven't seen that
[14:18:11] 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] i.e. the "overload" is bulkDocs(options: { docs: DocType[] }, callback?: CallbackType)
[14:19:57] (the overload in use in that test, that is)
[14:20:27] 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] nolanlawson: I'll keep working through them, just pushed the latest code to feat branch - you can see the overloads in 3110b77, 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
Copy link
Owner Author

AGBrown commented May 3, 2015

@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 3110b77 I started to add in the overloads to go with option (3), and that has now evolved into 3732dce. 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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants