feathers-vuex icon indicating copy to clipboard operation
feathers-vuex copied to clipboard

Enhance `paramsForServer` with parameters with specific values.

Open Yavorss opened this issue 3 years ago • 7 comments

Summary

This a solution to the issue with -1 from feathers-hooks-common mentioned here https://github.com/feathersjs-ecosystem/feathers-vuex/issues/590. The solution is simple. Instead of filtering only by parameter, we could filter by parameter with a specific value. For instance, paramsForServer: [['$limit', '-1']] or paramsForServer: [['$limit', value => value === '-1']]. This is not a breaking change.

Other Information

If that looks good for the team, we could update the documentation and link that PR to here as well.

Yavorss avatar Jul 02 '21 11:07 Yavorss

Please, take a look at my comment there

J3m5 avatar Jul 02 '21 11:07 J3m5

@J3m5 Allowing a filtering function within theparamsForServer solves the $limit: -1 issue and also adds some needed functionality to the paramsForServer filter.

That would be a more straightforward solution over hard-coding the -1 in multiple places within the code and enhances the utility of paramsForServer at the same time.

joshuaja avatar Jul 02 '21 15:07 joshuaja

Ok I get it.

There is one small problem however. This MR will conflict with a previous one

And I think the added code could be isolated in a separate function.

J3m5 avatar Jul 02 '21 16:07 J3m5

This is a good idea. Can you double check the conflicts from the other PRs merged?

marshallswain avatar Jul 03 '21 23:07 marshallswain

This is a good idea. Can you double check the conflicts from the other PRs merged?

Will update it these days or even today.

Yavorss avatar Jul 06 '21 09:07 Yavorss

Pretty nice! I also thought about this before.

A mention about this in the docs would be pretty neat! @Yavorss could you add this to the docs?

fratzinger avatar Jul 26 '21 08:07 fratzinger

@Yavorss were you able to make any additional progress on this. I should have some time soon to take a look, if not.

marshallswain avatar Oct 06 '21 13:10 marshallswain