n1ru4l/graphql-live-query

`createLiveQueryPatchGenerator` shouldn't yield empty patches

dburles opened this issue · 4 comments

dburles commented

Hey @n1ru4l thanks for all your hard work on these libraries!

I've come across an issue where I'm seeing a load of empty patches, e.g.: {"revision":92}, which the client isn't too happy about, https://github.com/n1ru4l/graphql-live-query/blob/main/packages/graphql-live-query-patch/src/createApplyLiveQueryPatch.ts#L42-L44.

The generatePatch function returns undefined in the case where previousValue is the same as currentValue. I believe there should be a check in place to not yield valueToPublish in those cases.

n1ru4l commented

@dburles Could you send a PR with a failing test (and ideally a fix)?

dburles commented

@n1ru4l Sure

  • 🎉1
  • 🚀1
dburles commented

Hey @n1ru4l, just spent some time looking into this, and I'm not totally sure on the solution. What I think is correct is basically, if invalidate is called, it should be a NOOP if generatePatch returns undefined (a match between previousValue and currentValue). The problem is that this needs to happen closer to invalidate, before it executes the query, since we can't simply escape from the iteration on asyncIterator within the liveQueryPatchGenerator as it breaks subsequent invalidations.

I don't think there's any way around the need to execute the query to determine what's changed. In that case I think the solution might be to escape from liveQueryPatchGenerator if say we yield undefined from liveQueryPatchGenerator to indicate a NOOP.

  • 👍1
dburles commented

PR incoming