apollo-client icon indicating copy to clipboard operation
apollo-client copied to clipboard

Apollo misses populating properties when fragments are spread

Open pastelsky opened this issue 5 years ago • 19 comments

Consider the following query —

  query GetMarketplaceAppListingById {
    marketplaceApp(appId: 1215460) {
      versions(first: 1) {
        edges {
          node {
            deployment {
              compatibleProducts {
                id
                ...Fragment1
              }
            }
          }
        }
      }
    }
  }

  fragment Fragment1 on CompatibleAtlassianProduct {
    __typename
    atlassianProduct {
      name
    }
  }

Here, If I spread Fragment1 instead of inlining properties, Apollo will not make the properties inside Fragment1 (e.g. atlassianProduct) available as data when using useQuery.

However, using the same query with a fetch POST request returns expected results.

image

Intended outcome: All fields inside the spread fragment should be visible in data

Actual outcome: All fields inside the spread fragment are ignored. However, If I inline properties of the fragment, expected results are obtained.

How to reproduce the issue: See minimal reproduction — https://codesandbox.io/s/apollo-graphql-fragment-bug-50yzl Versions 3.3.x

pastelsky avatar Feb 04 '21 05:02 pastelsky

@pastelsky The issue here is that Apollo Client does not automatically know that CompatibleAtlassianDataCenterProduct is a subtype of the fragment type condition CompatibleAtlassianProduct. If the type names matched exactly, everything would work, but when subtyping is involved, you need some additional cache configuration.

The way to configure this information is by passing a possibleTypes object to the InMemoryCache constructor:

const client = new ApolloClient({
  cache: new InMemoryCache({
    possibleTypes: {
      CompatibleAtlassianProduct: [
        // List of all known direct subtypes of CompatibleAtlassianProduct:
        "CompatibleAtlassianDataCenterProduct",
        // ...
      ],
    },
  }),
  uri: "https://api.atlassian.com/graphql"
});

While you can mostly get away with manually listing only the subtypes that you actually need, you might want to generate possibleTypes automatically from your schema. Here are some instructions for how you might set that up.

benjamn avatar Feb 04 '21 17:02 benjamn

Thanks @benjamn . I'll try this solution out. But regardless, I think it would be helpful to inform the user that a field type might not be resolvable using the currently available information and possibleTypes should be provided.

I found skipping of fields to be quite non-intuitive.

pastelsky avatar Feb 09 '21 17:02 pastelsky

@pastelsky I hear you, but giving a useful warning is difficult here, because you don't need possibleTypes if all your fragment type conditions are concrete type names (like CompatibleAtlassianDataCenterProduct instead of CompatibleAtlassianProduct), and the client can't automatically detect when a fragment type condition is referring to an abstract interface or union supertype (which is the case where we would like to warn if you don't have possibleTypes, or you don't include that supertype in your possibleTypes). If there was a good way to detect this situation, I'd be happy to warn about it.

benjamn avatar Feb 17 '21 18:02 benjamn

Hello, I might have a similar scenario.

I have an interface _Item that with this object implementations: LandingPage, ListingPage SimplePage.

If I run the introspection query:

{
  __schema {
    types {
      interfaces {
        name
        possibleTypes {name}
      }
    }
  }
}

I get

{
# ...
        {
          "name": "SimplePage", # same for ListingPage and LandingPage
          "kind": "OBJECT",
          "possibleTypes": null,
          "interfaces": [
            {
              "name": "_Item",
              "possibleTypes": [
                {
                  "name": "ListingPage"
                },
                {
                  "name": "LandingPage"
                },
                {
                  "name": "SimplePage"
                },
              ]
            }
          ]
        },
        # ...
        {
          "name": "_Homepage_Content",
          "kind": "UNION",
          "possibleTypes": [
            {
              "name": "LandingPage"
            },
            {
              "name": "SimplePage"
            }
          ],
          "interfaces": null
        },
# ...
}

I have a query the content property of type _Homepage_Content, which is a union type (see above in response).

{
  homepage(codename: "homepage") {
    content {
      ... on _Item {
        _system_ {
          name
          codename
        }
      }
    }
  }
}

Which is also in suggestions in GraphiQL (so the information should be in the scheme). image

Direct request returns the data, but Apollo Client does not.

EDIT: I have added the following configuration in the client initialization and it worked, but the scheme is dynamic possible types are changing, so this is not a long time work around.

const client = new ApolloClient({
    cache: new InMemoryCache({
        // https://github.com/apollographql/apollo-client/issues/7648
        possibleTypes: {
            _Item: [
                "LandingPage",
                "ListingPage",
                "SimplePage"
            ]
        }
    }),
    uri: `${GQL_ENDPOINT}/${PROJECT_ID}`,
});

Simply007 avatar Nov 15 '21 14:11 Simply007

I'm running into the same issue using inline fragments with an interface type. The query runs fine in graphiql or directly with the server, but not with apollo client. This is actually shown as an example in the union types section of graphql.org. Very similar to the example used for the documentation of possibleTypes.

I'm just not understanding why resolving an interface type is a concern of the client to begin with. The client is naturally going to be worse at resolving these types than the server, and it's unclear to me why it needs to.

I think it would help if the fragments section of the documentation gave an example of a feature where apollo client needs this information. In other words why can't apollo client just rely on the server and pass through the same data? What use cases are being affected by not having this information in the client?

Explaining the requirements of apollo also explains the need for manual configuration or a process to automate it. Especially when data is being discarded executing an example from graphql.org, but works querying the server directly.

rsimp avatar Feb 28 '22 21:02 rsimp

@benjamn

I too have the same issue. I am using "@apollo/client": "^3.7.10", The data is visible in the network tab but can't able to access it in the application.

Query:

const CREATED_BY = 
  fragment CreatedBy on Person {
        name
        jobTitle
        url
    }
;
const REVIEWED_BY = 
    fragment ReviewedBy on Person {
        name
        jobTitle
        url
    }
;
const getArticleQuery = (article: IArticleIdentifier) => ({
    document: 
${CREATED_BY}
${REVIEWED_BY}
query ArticleContentQuery($id: String!) {
    ${article.type}(where: { id: $id }) {
        text(urlKind: identifier)
        lastReviewed
        nextReviewed
        printableCopy
        headline
        version
        introduction
        creator {
            ...CreatedBy
        }
        reviewedBy {
            ...ReviewedBy
        }
        audience {
            audienceType
        }
        identifier(name: "MedicalAuthors") {
            value
        }
    }
}`,
    variables: { id: article.id },
});

This is appolo client object

const cache = new InMemoryCache({
    addTypename: false,
    typePolicies: {
        Query: {
            fields: {
                search: {
                    keyArgs: ["term", "audienceType", "type"],
                    // eslint-disable-next-line @typescript-eslint/default-param-last
                    merge(existing = { results: [] }, incoming) {
                        return { ...incoming, results: existing.results.concat(incoming.results) };
                    },
                },
            },
        },
    },
});

const useApolloClient = (accessToken?: string): ApolloClient<any> => {
    return new ApolloClient({
        link: authMiddleware(accessToken).concat(httpLink),
        cache,
    });
};

parithibang avatar Apr 20 '23 08:04 parithibang

@parithibang I assume that your creator is not of type Person, but of a type implementing that interface? As stated above you need to provide possibleTypes in that scenario.

phryneas avatar Apr 20 '23 09:04 phryneas

@phryneas

The creator type name is person only

creator {
    __typename
}
"creator": {
    "__typename": "Person"
},

So my possible types should be

const cache = new InMemoryCache({
    addTypename: false,
    possibleTypes: {
        Person: ["Person"],
    },
    typePolicies: {
        Query: {
            fields: {
                search: {
                    keyArgs: ["term", "audienceType", "type"],
                    // eslint-disable-next-line @typescript-eslint/default-param-last
                    merge(existing = { results: [] }, incoming) {
                        return { ...incoming, results: existing.results.concat(incoming.results) };
                    },
                },
            },
        },
    },
});
Screenshot 2023-04-20 at 3 48 20 PM

parithibang avatar Apr 20 '23 10:04 parithibang

@parithibang in that case, you do have a separate issue than the issue that was discussed here originally. Could you try to create a minimal reproduction for this and open a new issue?

phryneas avatar Apr 20 '23 10:04 phryneas

We are moving from a custom GraphQL client implementation (which does not mask fragment fields) to Apollo-client, and encountered this issue. I would like to echo @rsimp's arguments -- I am also not convinced that a GraphQL client should need to know possible types to resolve fragments in interfaces.

The previous documentation for Apollo client v2 has more statements on why clients wants supertype-subtype relationship information, although the solution back in v2 was using IntrospectionFragmentMatcher.

Coming from a much simpler GraphQL client, I am not convinced by the reasons inside the documentation. Specifically,

  • "Apollo Client cannot check the server response for you": GraphQL server implementations should already block the fields not defined in schema. Also, generated typescript types can correctly resolve the fields from the query with interface. I do not expect checking fields from server is apollo-client's duty.
  • "it cannot tell you when you're manually writing invalid data into the store using update, updateQuery, writeQuery": This may be the reason. It's just that our application don't need to mutate the store this way.

For workarounds, we have:

  • setting up possibleTypes as instructed in the documentation
  • avoid query fields under interface fragment, but repeating the reused fields in each individual types that implements the interface -- which makes the query super long.

I would like to know if it is possible to have an option that bypasses the fragment masking / matching behavior of useQuery.

johnson-liang-appier avatar Jun 06 '23 12:06 johnson-liang-appier

This was very surprising and confusing behavior, and my team lost hours bumbling around before I finally discovered this ticket.

I'm trying to understand what the issue is here. Is the core of it that the Client can't be assumed to have access to the an (accurate) schema, and therefore, there's no way for it to even know that a fragment is on an interface versus a concrete type, because it doesn't know the types it's querying against in the first place?

If so, I get why this is hard to automatically solve. Although it still does seem like Apollo Client should be able to detect when there is extra information that it is discarding. Could this made into be a warnable condition? Would it be possible to establish a norm of putting decorators on fragments that signal the possible subtypes of the queries that call them? Is it possible to detect this situation at build time when a schema is available?

This is a pretty nasty trapdoor, and it's unfortunate if it truly can't be made better than the status quo.

acjay avatar Oct 05 '24 21:10 acjay

Also, for future people, with GraphQL Codegen, you can automate this at build time. See this plugin: https://the-guild.dev/graphql/codegen/plugins/other/fragment-matcher

acjay avatar Oct 05 '24 22:10 acjay

Bumping this because I still don't understand why this can't at least be toggled via a setting somewhere. Without this, I am copying and pasting fragments everywhere because I have a unified interface that I want to spread a single multiplexed fragment across.

jamietdavidson avatar May 20 '25 20:05 jamietdavidson

@jamietdavidson what setting are you looking for? possibleTypes should be the solution to this, so what else do you have in mind?


For some of the comments above ☝, the reason it's so hard for Apollo Client to make an accurate predication on what to fragments to traverse is because it doesn't know the schema and therefore has no way to map supertypes (from interfaces and unions) to the possible subtypes without that information. You might ask "why not use the result of an introspection query" here and the reason is because very large schemas could be megabytes of data (we've seen schemas with thousands of types and hundreds of thousands of fields). If Apollo Client required that you give it introspection data, you are asking your users to download possibly megabytes of additional data every time you render your app so that Apollo Client can be a bit smarter. We prefer a small bit of configuration (e.g. possibleTypes) in lieu of an that introspection result that might be very large.

For those asking why we don't warn when detecting when information is discarded, its because its impossible to tell when we should actually warn since its perfectly valid to add a fragment spread on a type condition that isn't an exact match on the __typename.

Check out this section of the GraphQL spec in regards to fragments and type conditions:

Fragments can be specified on object types, interfaces, and unions.

Selections within fragments only return values when the concrete type of the object it is operating on matches the type of the fragment.

For example in this operation using the Facebook data model:

query FragmentTyping {
  profiles(handles: ["zuck", "coca-cola"]) {
    handle
    ...userFragment
    ...pageFragment
  }
}

fragment userFragment on User {
  friends {
    count
  }
}

fragment pageFragment on Page {
  likers {
    count
  }
}

In this example, profiles can return either User or Page items in its list. When profiles returns a User item, the Page fragment won't be traversed. Similarly, when the profiles field returns a Page item, the User fragment won't be traversed. Are fields "discarded" as a result of this? Yes, but thats because this is valid GraphQL and expected as according to the spec. If we warned in this situation, we'd quickly annoy users who are writing valid GraphQL queries according to the spec.

Again, because Apollo Client has no knowledge of your schema without the possibleTypes definition, it has no idea whether the __typename returned from an object should match a given type condition on a fragment unless they are an exact match. Given the example above, it would be perfectly fine for profiles to return an item of type Foo as well, in which case neither fragment would be traversed. Apollo Client is actually doing the right thing here according to the spec.

I definitely get the frustration and we apologize for those that spend an inordinate amount of time debugging this only to realize that possibleTypes needs to be configured, but its very difficult to determine how best to handle these situations while a) complying with the spec b) not warning unnecessarily and c) ensuring we don't force you to provide potentially megabytes of introspection data so that Apollo Client can be a little bit smarter.

We are all ears for ideas on how to better detect and warn for these situations, but I just wanted to highlight why this situation is so tricky and why it's not trivial to just "add a setting" or "add a warning" to fix this.

jerelmiller avatar May 20 '25 21:05 jerelmiller

@jerelmiller thanks for the explanation. I do understand that running an introspection query everytime is not feasible. The bigger question I still have, is why can't I enable a passthrough flag on interfaces? I have an interface that almost every major object in my graph implements, and a few highly generic mutations and queries (updateNode, createNode, deleteNodes) that returns IObject (which implements Node). Part of my build process is to write these backend objects into fragments that the frontend (and hence Apollo consumes). Ideally, I'd like this process to be streamlined.

Ex:

updateNode(input: UpdateNodeInput) { 
  data { 
    ...on IObject { 
      ...Object 
    }
  }
}

fragment Object on IObject {
  ...on BankAccountObject {
    ...BankAccount
  }
  ...on ChartedAccountObject {
    ...ChartedAccount
  }
  ...on ChatObject {
    ...Chat
  }
  ...
}

But instead, I have to add to possibleTypes everytime I add or remove a backend object. It's not the end of the world, it's just more scripting I have to do.

I do still struggle to understand why it can't be assumed that if the server returns something for this query, that it's a valid implementation of that interface? Would a flag like allowUnsafeServerResponsesOnInterfaces=true suffice?

jamietdavidson avatar May 20 '25 21:05 jamietdavidson

Would a flag like allowUnsafeServerResponsesOnInterfaces=true suffice?

We'd like to avoid this because if even one type doesn't implement that interface and you type a query wrong, you'll introduce other bugs that could be worse.

For example, Apollo Client treats the undefined value on a field as a cache miss. If we traverse the wrong fragment, we might end up with a field that gets a value set to undefined (by virtue that the type returned by the server might not actually implement that interface, therefore it won't have those fields in the response). When you're using cache-* fetch policies, those cache misses tells Apollo Client it needs to refetch that query in order to fulfill its data requirements. Those refetches might be endless in that case because after it refetches and tries to traverse that incorrect fragment again, it will get undefined for those fragment fields, which results in a cache miss, which results in a refetch, etc. That is going to be a far more expensive and frustrating mistake than just not traversing the fragment in first place. And you can imagine the kind of angry issues we'd get opened about that problem 🤣

You'd need to be very careful if you were to use something like allowUnsafeServerResponsesOnInterfaces again because all it takes is for one object type not to implement that interface and to add that fragment to a query somewhere for this problem to show up.

As mentioned in one of the other comments, GraphQL Codegen already has a fragment-matcher plugin that will generate those possibleTypes for you. If you prefer automating it, it should be feasible to hook this up to CI and commit that in your PRs so you don't have to remember to manually run this. It's definitely better than trying to manually configure possibleTypes yourself. We'd prefer sticking to this if you can. Again, I know its not the most ideal and slightly annoying, but we don't have a better solution at the moment that would be safe enough to avoid the situation I mentioned above.

Hope that helps 🙂

jerelmiller avatar May 20 '25 21:05 jerelmiller

Hey all 👋

Quick update. We had a team meeting this morning about this and we think that adding a warning if we detect that possibleTypes aren't configured and we are about to skip traversing a fragment might help here. In that case, adding possibleTypes: {} would be enough to silence the warning, but should provide a helpful hint that the cache might be misconfigured and you might see some missing data in your response.

We are hard at work for 4.0 right now so we don't have the capacity to work on this quite yet. If anyone is interested and would like to contribute this change, we'd be happy to accept a PR!

jerelmiller avatar May 21 '25 17:05 jerelmiller

@jerelmiller I would like to contribute to this , faced this issue in prod environment and displaying warning would be useful to devs in their local environment .

Also how can we use fragment-matcher plugin to solve the use case for generating possibleTypes file ? If there is any documentation around this @benjamn @jerelmiller

DIVANSHMAHAJAN avatar Jun 11 '25 12:06 DIVANSHMAHAJAN

@DIVANSHMAHAJAN feel free to open a PR!

For the fragment matcher plugin, see https://www.apollographql.com/docs/react/data/fragments#generating-possibletypes-with-graphql-codegen. The fragment matcher plugin documentation you reference also has a section at the bottom called "How to use" that shows how to use it with Apollo Client:

Image

jerelmiller avatar Jun 11 '25 15:06 jerelmiller