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

Calling methods on resources with arg list throws error #270

Closed
wooldridge opened this issue Aug 10, 2016 · 7 comments
Closed

Calling methods on resources with arg list throws error #270

wooldridge opened this issue Aug 10, 2016 · 7 comments
Assignees
Milestone

Comments

@wooldridge
Copy link
Contributor

If you call the methods on resources:

get()
post()
put()
remove()

with a list of arguments, e.g.:

db.resources.get('testService', {a: 1, b: 2})
  .result(function(response){
    console.log(JSON.stringify(response, null, 2));
  });

an error is thrown:

Error: no name for executing resource service

When arguments are passed in as a single object, all is OK, e.g.:

db.resources.get({name: 'testService', params: {a: 1, b: 2}})
  .result(function(response){
    console.log(JSON.stringify(response, null, 2));
  });
@wooldridge wooldridge added this to the 9.0-ea4 milestone Aug 10, 2016
@wooldridge wooldridge self-assigned this Aug 10, 2016
@kcoleman-marklogic
Copy link
Contributor

Isn't that WAI? At least as far as requiring you to use a call obj if you want to pass in params. The error message could certainly be better. :)

That is the usage I documented, at any rate. It's also consistent with the spec from 8.0, which shows this usage:

db.resources.get({name:..., params:{...}, txid:...})

This is pretty typical of the API, IIRC. You have to fallback on a call object once you stray from the bone simple use case.

@wooldridge
Copy link
Contributor Author

Thanks for the input, @kcoleman-marklogic. Your example from the spec makes the expected usage very clear to me:

db.resources.get({name:..., params:{...}, txid:...})

This signature from the jsdocs tells me I should do something different:

get(name, params, txid) -> {ResultProvider}

This may be a limitation of jsdocs.

For now, I'm going to push this issue out because for me it speaks to making it clearer across the docs that the call object is the expected usage.

I also see that the issue probably can't be "fixed" as I had thought (i.e., make positional params always possible) because of the mix of optional parameter data types in some of the methods.

@wooldridge wooldridge modified the milestones: 9.0-1, 9.0-ea4 Oct 25, 2016
@kcoleman-marklogic
Copy link
Contributor

FWIW, I think there are some other places where we try to finesse this limitation with the JSDoc through words.

For example, take a look at Documents.remove: http://docs.marklogic.com/jsdoc/documents.html#remove. There, we say "... takes a configuration object with the following named parameters or, as a shortcut, one or more uri strings or an array of uri strings".

Of course, that pre-supposes you understand that "named parameters" means a call object. I have no idea if that's a safe assumption for a Node.js developer or not.

A lot of this could be mitigated by including simple examples of the accepted call forms in the JSDoc. IDK how hard or easy that would be as I'm not very conversant with the markup. Erik got saddled with all that work back at the beginning.

@wooldridge
Copy link
Contributor Author

@kcoleman-marklogic
Copy link
Contributor

Great find! I think that has a lot of potential to address some of the call object confusion. It probably still can't help with the "call object or these shortcuts" problem, but it is certainly worth experimenting with.

@wooldridge wooldridge modified the milestones: 9.0-2, 9.0-1 Feb 22, 2017
@wooldridge wooldridge modified the milestones: 9.0-2, 9.0-3 May 15, 2017
@wooldridge wooldridge modified the milestones: 9.0-4, 9.0-3 Jul 27, 2017
@wooldridge wooldridge modified the milestones: 2.1.1, 3.0.1 Nov 16, 2017
@georgeajit georgeajit modified the milestones: 3.0.1, 2.4.next Apr 30, 2020
@georgeajit georgeajit assigned ehennum and unassigned wooldridge Apr 30, 2020
@ehennum ehennum modified the milestones: 2.5.0, 2.5-NEXT Nov 16, 2020
@anu3990 anu3990 added minor and removed major labels Apr 28, 2021
ehennum pushed a commit that referenced this issue Oct 4, 2021
@ehennum
Copy link
Contributor

ehennum commented Oct 4, 2021

Added an explanatory sentence to the JSDoc for each of the resource execution methods.

Given that Data Services endpoints offer a better way to execute code on the server, it's better not to risk destabilizing the implementation by changing the argument parsing.

@ehennum ehennum assigned georgeajit and unassigned ehennum Oct 4, 2021
@ehennum ehennum added test and removed new labels Oct 4, 2021
@ehennum
Copy link
Contributor

ehennum commented Oct 4, 2021

Should cause no regressions.

@georgeajit georgeajit added ship and removed test labels Oct 27, 2021
@anu3990 anu3990 modified the milestones: node-client-api-NEXT, 2.8.0 Feb 15, 2022
@anu3990 anu3990 closed this as completed Feb 15, 2022
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

5 participants