convoyinc / apollo-cache-hermes

A cache implementation for Apollo Client, tuned for performance
Other
483 stars 30 forks source link

@connection directive support #289

Open bennyhobart opened 6 years ago

bennyhobart commented 6 years ago

Hi guys, I'm not sure if this is an issue with my application but it seems like the @connection directive isn't working as expected

the two places I use it are as such, (code stripped for brevity)

activities(first: 30) @connection(key: "activities")
activities(first: 30, after: $cursor) @connection(key: "activities") 

If I remove the cursor from the second query it is working fine, any ideas?

nevir commented 6 years ago

Ah, hmm, this cache doesn't have explicit support for @connection—though, IIRC, most of that should be handled by Apollo client explicitly.

I'm curious: what keys end up being written to when you load the first query as opposed to the second? Try turning on verbose mode, which'll log out snapshots for each write and lists of ids that have changed; might help us narrow it down

bennyhobart commented 6 years ago

@nevirI picked this out from the logs, it looks like they are being stored in two different locations

this is the root log screen shot 2018-01-16 at 2 06 42 pm

this is the important part screen shot 2018-01-16 at 2 06 08 pm

Any action I can take to help?

bennyhobart commented 6 years ago

I think at this point https://github.com/convoyinc/apollo-cache-hermes/blob/5e2b7604fec684cd27ffd9a585e706a704415c4e/src/operations/SnapshotEditor.ts#L279 We determine if the field is a connection directive? and change the key based on that

nevir commented 6 years ago

Yeah, or potentially earlier when parsing the query (ParsedQueryNode) - however, we also have to teach the cache how to combine the two lists too (maybe append by default, but that quickly falls over), right?

nevir commented 6 years ago

Re:

Any action I can take to help?

We're unfortunately not going to have much bandwidth to dig into this much for the next couple weeks—if you want to take a stab at an implementation, that'd be a huge help! It might also be worth checking out how inmemory handles the @connection directive

bennyhobart commented 6 years ago

I'll take a stab when I get the opportunity, we should avoid merging there isn't enough information to do so, overwrite the previous result with the new which will keep inserts fast, it's the responsibility of the application developer — when fetching more) — to maintain this state, thanks for the research into in-memory cache's implementation