figbird icon indicating copy to clipboard operation
figbird copied to clipboard

Invalid Query Parameter - `useFind` affects `useMutation` methods

Open robbyphillips opened this issue 4 years ago • 3 comments

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.

robbyphillips avatar Oct 14 '20 16:10 robbyphillips

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.

KidkArolis avatar Oct 19 '20 21:10 KidkArolis

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?

robbyphillips avatar Oct 19 '20 22:10 robbyphillips

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.

KidkArolis avatar Oct 20 '20 14:10 KidkArolis