Separate "query" from other parameters in service calls

[hubgit]

It's common to pass a query to the find service method, but less common to pass any other parameters, so for convenience perhaps query should be separated from params in service calls.

class MyService {
  find(params) {}
  get(id, params) {}
  create(data, params) {}
  update(id, data, params) {}
  patch(id, data, params) {}
  remove(id, params) {}
  setup(app, path) {}
}

would become

class MyService {
  find(query, params) {}
  get(id, params) {}
  create(data, params) {}
  update(id/query, data, params) {}
  patch(id/query, data, params) {}
  remove(id/query, params) {}
  setup(app, path) {}
}

This is obviously a breaking change, but now seems as good a time as any to consider implementing it (or not).

Previous discussion:

• https://github.com/feathersjs/feathers-mongoose/issues/74#issuecomment-204615610
• https://github.com/feathersjs/feathers-mongoose/issues/74#issuecomment-212697450

[marshallswain]

Same for get.

get(id/query, params) {}

I like this idea, but @daffl will have thought this through more than I.

[daffl]

Yeah, @ekryski has been a big proponent of this change, too. The only tricky part would be backwards compatibility with old services. I am not sure there is a way to feature detect this at all (maybe with the method arity of find via service.find.length but it won't work if someone is still using callbacks - which we really should throw a warning or an error by now).

It would definitely help clear up the confusion about which parameters are passed between the server and the client (and mistakes of doing a .find all accidentally).

[ekryski]

I was literally just thinking about this today. This has come up a lot and I've also been burned a few times by the existing query syntax.

I'm going to create an issue for a proposal as this definitely would be MASSIVE breaking change and effects just about everything. However...

I think we could create a hook that you can include that would be able to map a new hook format back to the legacy one and we'd be able to roll out new major versions of adapters.

The next release would be the time to do it as we are planning on bringing feathers-hooks into core feathers itself.

[eddyystop]

I have doubts if this mainly visual change is worth the problems its going to cause.

[idibidiart]

I think 'params' is overloaded.

It contains 'provider' and 'token' which should be split up under a 'context' argument

It also contains 'query' in case of find()

It also contains an array of ids in case of patch() and remove() ... not sure about update() ... I don't even recall how to supply the ids in params but maybe id: [...ids] ?

I think we should split into query, context and id array.

Noob comment. Just getting back to Feathers.

[idibidiart]

Or id argument can be string or array

so:

id (string or array) query (object) context (object)

params as it is is too overrloaded and hard to remember the details