mattkrick / cashay

:moneybag: Relay for the rest of us :moneybag:
MIT License
453 stars 28 forks source link

Cache went stale and wasn't recreated - explanation #168

Open theobat opened 7 years ago

theobat commented 7 years ago

So I experienced the issue pointed out by @dustinfarris, the culprit is here.

The reason why this "invalidation" cause the "cache went stale..." exception is that the flushDependencies function essentially walk the path of denormalizedDeps and find all the dependencies in common with the denormalized response of the server. Then for all the ops(key) in denormalizedDeps, it removes the response in cachedResponse. Practically speaking, if I have thingA, thingB fetched by getAB and thingD, thingC fetched by getDC, and if A and D are owned by personA (thing and person beeing graphql Types) then we'll have an issue if:

@mattkrick I know you don't have time these days, but perhaps a small hint as to why the flush dependencies is important and more specifically why not:

I can see numerous solutions to this issue but I don't use subscriptions yet and I'd like to make a fix that maintain the subscriptions part unchanged (if there's no bug there of course).

mattkrick commented 7 years ago

a dependency links the op (usually your viewmodel container) to the model. without flushing deps, your app won't ever know what got updated, and could update when one isn't required.

is your mutationhandler referencing all the ops that it will mutate? if not, that's usually how it goes stale (the mutation updates the underlying data, but the query hasn't been updated by the mutation).

the cachedResponse will always get removed if any of the underlying doc fields get changed. if we didn't cache, then the object would get recreated every time dispatch got called.

theobat commented 7 years ago

is your mutationhandler referencing all the ops that it will mutate? if not, that's usually how it goes stale (the mutation updates the underlying data, but the query hasn't been updated by the mutation).

Well the thing is I don't use mutation as a mean to return data in this case, I only use the invalidate function in the mutationHandler (which by the way is not working on master, and I made a PR to solve that, the 'this' keyword does not refer to the singleton which means this._willInvalidate... is never set to true). This is brutal in a sense, but should get the job done while I work on better mutation handling.

the cachedResponse will always get removed if any of the underlying doc fields get changed. if we didn't cache, then the object would get recreated every time dispatch got called.

So the flushDependencies function removes the cachedResponse of a query that just fired... Meaning that if I query a list of post, get into a post detail view (so the list is not "observed" anymore it's in a different view) modify this post with a mutation that triggers a mutationHandler on the list query, it will trigger the exception (no matter what this mutationHandler does btw, since this happens after the exception). Will try to isolate a test case for you to grasp what's going on.

mattkrick commented 7 years ago

yeah, an example might be good. whenever dispatch is called, all the invalidated queries will get recalculated.