search

apollographql / apollo-ios

📱  A strongly-typed, caching GraphQL client for iOS, written in Swift.
https://www.apollographql.com/docs/ios/
MIT License
3.87k stars 717 forks source link

`_asInlineFragment` always evaluating to `true` without direct selections #3309

Closed spencer-coterie closed 8 months ago

spencer-coterie commented 8 months ago

Question

After upgrading to the latest version, 1.7.1, I'm seeing a change in the behavior which I believe to be caused by the introduction of CompositeInlineFragment. With this change, the _asInlineFragment() is evaluating to true for all types which have no direct selections of its own.

We commonly employ a pattern where the GQL union is compromised of multiple types, and it's common for at least 1 type to have no direct selections.

We previously used the asMyCustomType variable as a way to identify the type, but this appears to be falsely evaluating to true, which is a change in behavior. I believe this change was first introduced in version 1.1. We previously pinned the version to 1.0.7 and were not seeing this issue.

Workaround

For the time being, we have a workaround, and that's to perform a direct comparison with __typename.

Questions

calvincestari commented 8 months ago

Hi @spencer-coterie

After upgrading to the latest version, 1.7.1, I'm seeing a change in the behavior which I believe to be caused by the introduction of CompositeInlineFragment. With this change, the _asInlineFragment() is evaluating to true for all types which have no direct selections of its own.

I believe this is correct behaviour. _asInlineFragment() is not evaluated on whether there are any direct selections but rather whether the returned type corresponds to the inline fragment type. If you look at the condition for _asInlineFragment() it looks at whether the fragment has been 'fulfilled', i.e.: returned in the response.

We previously used the asMyCustomType variable as a way to identify the type, but this appears to be falsely evaluating to true, which is a change in behavior. I believe this change was first introduced in version 1.1. We previously pinned the version to 1.0.7 and were not seeing this issue.

Even though your operation may not have any direct selections for the fragment Apollo iOS automatically inserts the __typename field so that we can correctly evaluate response types. So there will always be at least that field in the response. You mention 1.1 as being where you noticed the change which aligns with the following in the 1.1.0 release notes:

Changed

Generate __typename selection for generated models: In 1.1, the code generator adds the __typename field to each root object. In previous versions, this selection was automatically inferred by the GraphQLExecutor, however generating it directly should improve performance of GraphQL execution.

If it is intentional, is it possible to opt out via the configuration?

No, unfortunately this is not optional behaviour. _asInlineFragment() is correct and will allow you to evaluate what type is being returned but the assumption that no direct selections means it's not of that type is possibly the incorrect inference.

spencer-coterie commented 8 months ago

Thanks for the detailed reply @calvincestari - really appreciate it!

calvincestari commented 7 months ago

@spencer-coterie - I've recently made a fix regarding _asInlineFragment which is probably related to the behaviour you were experiencing in this issue. The explanation I gave in my previous comment here is still correct but I don't think we'd arrived at there actually being a bug. The other issue had a reproducible pattern that we could identify and fix.