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

Cache redirects don't work with interfaces specified by possibleTypes #382

Open
bluepichu opened this issue Mar 9, 2023 · 2 comments
Open
Labels
📕 cache Feature requests related to the cache

Comments

@bluepichu
Copy link

Issue Description

I asked this question on the Discord server and got no response. I think this is a bug, but it's possible I'm misunderstanding if/how these two features are supposed to interact.

I have a schema that looks like this:

type Query {
    widgets: [Widget!]!
    widget(id: ID!): Widget
}

interface Widget {
    id: ID!
    name: String!
}

type FooWidget implements Widget {
    id: ID!
    name: String!
    foo: String!
}

type BarWidget implements Widget {
    id: ID!
    name: String!
    bar: String!
}

I have a page where I'm fetching a full list of available widgets via query { widgets { id name } }, and then rendering the list of widgets. However, the "widget card" component used within that list is written in such a way that it can be reused in multiple locations, possibly including some where we don't already have the relevant data, and therefore is in charge of fetching its own data via query($id: ID!) { widget(id: $id) { id name } }. Naturally, this means that we end up round-tripping once for the list and then again for each list item.

The documented solution to this is to use a cache redirect, so I've set up my cache like this:

const apolloCache = new InMemoryCache({
    typePolicies: {
        Query: {
            fields: {
                widget: {
                    read(_, { args, toReference }) {
                        return toReference({
                            __typename: "Widget",
                            id: args.id
                        });
                    }
                }
            }
        }
    },
    possibleTypes: {
        Widget: ["FooWidget", "BarWidget"]
    }
});

My understanding is that this should allow the individual cards to fetch their data directly from the cache since all of the data they're requesting is already being loaded as part of the list query. However, I still see a second network request for each card going through. Stepping through the code in the developer tools, I noticed that when id is abc, then Apollo is reaching an error state because there is no value in the cache with key Widget:abc (in particular, the message Dangling reference to missing Widget:abc object is being propagated up to ObservableQuery.getCurrentResult). This is because the normalized key is FooWidget:abc since that's the actual concrete type of the widget in question. It seems like the possibleTypes configuration is not being respected with regards to checking for that key in the cache.

Link to Reproduction

https://github.com/bluepichu/react-apollo-cache-issue

Reproduction Steps

  1. Load the page
  2. Observe that the person list immediately jumps from the base "loading" state to a complete list, while the widget list goes through a step in the middle where the individual items are loading
  3. Open the logs
  4. Observe that the people data (which doesn't use interfaces at all) was only loaded once, while the widget data (which does use interfaces with possibleTypes) was loaded both as a complete list and as the individual items, despite the fact that the list query should provide sufficient information for the individual item queries to use the cache
@bluepichu bluepichu changed the title Cache redirects doesn't work with interfaces specified by possibleTypes Cache redirects don't work with interfaces specified by possibleTypes Mar 9, 2023
@bignimbus
Copy link
Contributor

Hi @bluepichu 👋🏻 thanks for opening this issue and thanks for your patience. First, it's totally understandable that a user would expect possibleTypes to factor into a toReference so thanks for this feedback. I believe that updating the default behavior would be a breaking change so I don't see that happening in version 3.x. That said, this is an interesting idea and something we want to explore further 🙏🏻

@bluepichu
Copy link
Author

bluepichu commented Mar 12, 2023

Thanks for getting back to me!

I think this is definitely worth looking into in the future since the expectation that toReference would work with inheritance is fairly reasonable, but in the meantime I've found a suitable workaround: using the keyFields type policy to customize the cache id. In my example repo, I just made this diff:

diff --git a/src/index.jsx b/src/index.jsx
index afc7a5f..9a2e416 100644
--- a/src/index.jsx
+++ b/src/index.jsx
@@ -127,6 +127,9 @@ const client = new ApolloClient({
             }
           }
         }
+      },
+      Widget: {
+        keyFields: (object) => `Widget:${object.id}`
       }
     },
     possibleTypes: {

Due to the typePolicy inheritance enabled by possibleTypes, FooWidget and BarWidget inherit this keyFields configuration and have cache keys of the form Widget:<id> instead of FooWidget:<id> or BarWidget:<id>. This makes the cache lookups resolve properly, and in my case is sufficient because it's guaranteed that widgets (even those of different types) will never have conflicting ids.

That said, it wasn't particularly obvious from the documentation or the name of the field that you could do this with keyFields; the docs made it sound like this behavior could only be accomplished via dataIdFromObject. I only figured this out because I was poking around in the TS types while checking the type of something unrelated. It might be good to add an example of using a function to fully specify the cache key in this section of the documentation, and might even be worth considering changing the name of the field in a future version since in this case the function generates the cache key directly rather than having anything in particular to do with fields of the object as the name suggests.

@alessbell alessbell transferred this issue from apollographql/apollo-client Apr 28, 2023
@alessbell alessbell added the project-apollo-client (legacy) LEGACY TAG DO NOT USE label Apr 28, 2023
@jerelmiller jerelmiller added 📕 cache Feature requests related to the cache and removed project-apollo-client (legacy) LEGACY TAG DO NOT USE labels Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📕 cache Feature requests related to the cache
Projects
None yet
Development

No branches or pull requests

4 participants