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

Querying for data already in the store with a different query #332

Closed
joarwilk opened this issue Jun 28, 2016 · 10 comments
Closed

Querying for data already in the store with a different query #332

joarwilk opened this issue Jun 28, 2016 · 10 comments
Milestone

Comments

@joarwilk
Copy link

These two queries share the same return type (the first is a list of events, the second is a single event). If I would first run the getFeed query and then run getEvent with an id that I retrieved in the getFeed query, the getEvent query would run and fetch both id and title even though that information already has been retrieved. In addition, it will update the same item in the store that getFeed did, causing the net effect of the query to be zero.

While the getEvent query is running, the user can't access the data already available from getFeed.

query getFeed {
    viewer {
        feed {
            id
            title
        }
    }
}
query getEvent($id: String!) {
    viewer {
        event(id: $id) {
            id
            title
        }
    }
}

Ideally, it would recognise that getEvent will only return an Event and first check the store for the existence of an item with that Type/ID-key.

@deoqc
Copy link

deoqc commented Jun 30, 2016

continuing this discussion:

I don't agree that all of what Relay specifies needs to be idiomatic GraphQL, especially because a lot of things like the mutation spec is simply driven by the idiosyncrasies of the Facebook internal GraphQL layer.

Totally agree and number one problem with Relay was the way mutation were treated. I didn't mean every aspect of Relay were idiomatic GraphQL, but the Good Parts like Connection, Node Interface and Global Id.

@stubailo
Copy link
Contributor

@deoqc totally agree. Have you seen this: http://docs.apollostack.com/apollo-client/how-it-works.html#normalize

Would you be interested in coming up with a design/implementation for making queries that can be fetched from already cached data do so via the dataId? Ideally it would be something that would support both Node interface queries, and the query specified by @joarwilk above.

Maybe it can be some sort of query middleware that would transform queries before reading from the cache, so we could ship a package that would identify Node queries, and convert them to a dataId and a SelectionSet.

Then we can simply call readSelectionSetFromStore: https://github.com/apollostack/apollo-client/blob/c9ea5bcb733aba90bc3c4e98878948d74022e2c4/src/data/readFromStore.ts#L70

@deoqc
Copy link

deoqc commented Jun 30, 2016

Nice docs @stubailo, I had missed these updates.

I will dive deeper in this over the weekend.

@stubailo stubailo changed the title Client sometimes queries for data already in the store Querying for data already in the store with a different query Jun 30, 2016
@nevir
Copy link
Contributor

nevir commented Jul 10, 2016

@stubailo I'm not sure it's feasible to mutate the SelectionSet that is used when fetching data from the store. We need to rewrite the value returned by the store, not the key used to look it up. For example:

Say after fetching the feed, the store has:

getFeed.viewer.feed: [1, 2]
1: {id: 1, title: "One"}
2: {id: 2, title: "Two"}

In order to satisfy getEvent(id:1) from the cache, the store needs to yield the id 1 as a value (which doesn't exist in any place we can rewrite to)

I think that implies that we need a middleware that can rewrite or supercede storeKeyNameFromField

nevir added a commit to convoyinc/apollo-client that referenced this issue Jul 10, 2016
By allowing users to rewrite data lookups from the store, they can satisfy behavior like apollographql#332
@nevir
Copy link
Contributor

nevir commented Jul 10, 2016

@deoqc @stubailo here's an approach to tackle this: #376

nevir added a commit to convoyinc/apollo-client that referenced this issue Jul 11, 2016
By allowing users to rewrite data lookups from the store, they can satisfy behavior like apollographql#332
nevir added a commit to convoyinc/apollo-client that referenced this issue Jul 11, 2016
By allowing users to rewrite data lookups from the store, they can satisfy behavior like apollographql#332
nevir added a commit to convoyinc/apollo-client that referenced this issue Jul 11, 2016
By allowing users to rewrite data lookups from the store, they can satisfy behavior like apollographql#332
nevir added a commit to convoyinc/apollo-client that referenced this issue Jul 14, 2016
By allowing users to rewrite data lookups from the store, they can satisfy behavior like apollographql#332
nevir added a commit to convoyinc/apollo-client that referenced this issue Jul 14, 2016
By allowing users to rewrite data lookups from the store, they can satisfy behavior like apollographql#332
nevir added a commit to convoyinc/apollo-client that referenced this issue Jul 14, 2016
By allowing users to rewrite data lookups from the store, they can satisfy behavior like apollographql#332
nevir added a commit to convoyinc/apollo-client that referenced this issue Jul 21, 2016
By allowing users to rewrite data lookups from the store, they can satisfy behavior like apollographql#332
nevir added a commit to convoyinc/apollo-client that referenced this issue Jul 21, 2016
By allowing users to rewrite data lookups from the store, they can satisfy behavior like apollographql#332
nevir added a commit to convoyinc/apollo-client that referenced this issue Jul 22, 2016
By allowing users to rewrite data lookups from the store, they can satisfy behavior like apollographql#332
nevir added a commit to convoyinc/apollo-client that referenced this issue Jul 25, 2016
By allowing users to rewrite data lookups from the store, they can satisfy behavior like apollographql#332
nevir added a commit to convoyinc/apollo-client that referenced this issue Jul 25, 2016
By allowing users to rewrite data lookups from the store, they can satisfy behavior like apollographql#332
nevir added a commit to convoyinc/apollo-client that referenced this issue Jul 26, 2016
By allowing users to rewrite data lookups from the store, they can satisfy behavior like apollographql#332
RylanH pushed a commit to convoyinc/apollo-client that referenced this issue Aug 16, 2016
By allowing users to rewrite data lookups from the store, they can satisfy behavior like apollographql#332
nevir added a commit to convoyinc/apollo-client that referenced this issue Aug 16, 2016
By allowing users to rewrite data lookups from the store, they can satisfy behavior like apollographql#332
nevir added a commit to convoyinc/apollo-client that referenced this issue Aug 16, 2016
By allowing users to rewrite data lookups from the store, they can satisfy behavior like apollographql#332
@helfer helfer added this to the New API/Refactor milestone Sep 29, 2016
@helfer helfer removed this from the Release 0.5 milestone Oct 26, 2016
@helfer helfer modified the milestones: Post 0.5, Release 0.5 Oct 26, 2016
@migueloller
Copy link

It's worth noting that now it's possible to achieve more efficient caching with the new result reducer feature. You can listen for actions on the getEvents query and appropriately update the getEvent query. It's not automatic, but it's possible. 😄

@helfer
Copy link
Contributor

helfer commented Nov 17, 2016

Implemented in #921, documentation to follow shortly.

@insidewhy
Copy link

@helfer cache redirects just don't seem to work for me.

I have accounts where each entry has a from field and then in another query I access account (id) { from, ...otherFields } but every time apollo re-requests account(id) { from, ...otherFields } when I'd expect to just see ...otherFields inside of that. My server checks whether from is being requested and if it is it there it causes some extra processing that isn't necessary.

@joarwilk
Copy link
Author

@ohjames They removed query diffing in 0.5, if that's what youre referring to.

https://dev-blog.apollodata.com/apollo-client-0-5-f1eb3f122ace

@insidewhy
Copy link

@joarwilk Thanks for letting me know! I can work around it with a slightly dirtier schema. Relay modern docs make a big deal about static queries but I'm not sure if it supports this level of query diffing, anyone else know?

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants