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

Invalid Query Parameter - useFind affects useMutation methods #11

Open
robbyphillips opened this issue Oct 14, 2020 · 3 comments
Open

Comments

@robbyphillips
Copy link

robbyphillips commented Oct 14, 2020

Repo:

https://github.com/robbyphillips/figbird-issue

Expected

Query parameters in useFind should have no effect on commands sent with useMutation.

Actual

Using a query operator that is not in this list in a useFind query causes an Invalid Query Parameter error when using a useMutation method later

To Reproduce (in the repo above)

  1. yarn install
  2. yarn start:dev
  3. Try to use the very search and create inputs.
  4. See the errors in the console.

Note that there is no error when using the query parameter $like in a useFind query. The error only occurs when using a method of useMutation, and even then, the actual create or patch is successful despite the error.

@KidkArolis
Copy link
Collaborator

At the core, the issue is that $like is not part of standard Feathers operators, and is also not supported out of the box by figbird or sift - the underlying library used for client side query matching.

To answer you question of why useMutation and useFind are dependant and have an effect on each other - that's because in figbird useFind and useGet are "live queries". This is what happens in more detail:

  • useMutation is used to create an item
  • this immediately triggers a "cache" update in all useGet and useFind queries currently being used or used in the past
  • this cache update is done to see if any of these queries should include the newly created item
  • this is done by matching the newly created item against the query passed to useFind
  • this works out of the box with any of the "standard" Feathers operators: $in, $nin, $lt, $lte, $gt, $gte, $ne, $or

The same behaviour happens with update/remove, not just create. The same happens when Feathers server sends realtime events, so this live query behaviour works not only when you create something locally, but also if other users / devices / tabs create something and the app receives a realtime event.

I hope that makes sense so far.

The issue is that you used $like which works on the server, because the feathers-objection plugin implements it and other extra operators. I thought this would be relatively easy to fix, but unfortunately, sift, which is used client side for query matching does not support $like out of the box, so the code to add this extra operator is a bit involved: https://github.com/robbyphillips/figbird-issue/pull/1/files

Another approach could be to opt out of the default realtime: 'merge' live query behaviour and instead use realtime: 'refetch'. This would not run query matching client side and would avoid the need to re-implement operators. Instead this would refetch any mounted queries when things are created/updated/removed: https://github.com/robbyphillips/figbird-issue/pull/2/files. Note, this doesn't work in this example repo, because it does not emit realtime events from what I could see. This means that when you create items, the list in the UI will not automagically update (because currently there are no realtime effects getting sent by the server).

There is another issue this example pointed out, although, perhaps not new - you implemented a useFind with a dynamic $like query that gets updated on every key press. This creates many queries in cache that currently don't ever get deleted by figbird, this will work ok, but if users typed many search queries that would inevitably lead to cache getting bloated and slow things down. I think that is something that would get solved once I get around adding cache eviction control/policies.

@robbyphillips
Copy link
Author

Thank you so much for the detailed response. This helps a ton.

The part that I was missing to debug this on my own was the role of the matcher function. Now that you point it out, it makes a lot more sense.

And you're totally right, I forgot to enable event emitting back to the client. My actual application does this, and the live updating stuff is awesome.

There is another issue this example pointed out, although, perhaps not new - you implemented a useFind with a dynamic $like query that gets updated on every key press. This creates many queries in cache that currently don't ever get deleted by figbird, this will work ok, but if users typed many search queries that would inevitably lead to cache getting bloated and slow things down. I think that is something that would get solved once I get around adding cache eviction control/policies.

Yes, this was just a really naive version without debouncing or anything, but that's a really interesting point that I didn't think about with the bloated cache.

Searching with $like was really meant to be a stopgap to get something working quickly. Do I understand correctly that using refetch only caches locally to that component, so that cache would be cleared each time it is unmounted?

I suppose the right way to handle search in the short term is to just do it outside figbird?

@KidkArolis
Copy link
Collaborator

Do I understand correctly that using refetch only caches locally to that component, so that cache would be cleared each time it is unmounted?

The refetch mode uses the same cache, so things will stick around forever in cache. Cache control / eviction is something I'm yet to add to this library. I was thinking about manual (you pick what to remove when), unmount (remove on unmount), ttl (remove on unmount after some delay). Caching is nice, because the way it currently works is if you revisit the same page with the same useFinds, you immediately see the data (and it gets refetched in the background) and it usually is the up to date data since it's updating in the background when realtime events come in (!).

Yeah, search is definitely an interesting use case in that it can be highly dynamic (generating many cache entries). One option would indeed be to do it outside figbird, why not. Another option would be go into internals and delete cache manually (scary :D). Yeah, I'll keep this issue open for a while as a reminder to think about these things.

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

No branches or pull requests

2 participants