[ApolloPagination] Update PaginationOutput to operate over GraphQLResults

[Iron-Ham]

Closes: apollographql/apollo-ios#3413

Caution

This is a breaking change.

The goal of this PR is two-fold:

1. We want to expose errors per-response when fetching data, and handle those errors gracefully where we can. While we do currently expose networking failures to consumers, we are not correctly parsing GraphQL error responses. Additionally, since we were not considering errors within the responses previously, we weren't handling them correctly. This was especially noticeable in loadAll, where some cases may lead to continued retries of fetching data.
2. We want to simplify usage.

Changes

Broken down into:

• Exposing Errors
• Simplifying Usage
• Other Changes

Exposing Errors

Changing the stored values of PaginationOutput

We started by changing the stored values of PaginationOutput to be GraphQLResult<GraphQLQuery.Data, instead of the GraphQLQuery.Data: See this diff for detail.

Changing the Output types of GraphQLQueryPager

With the changes to PaginationOutput in place, with each page containing a Source (i.e., cache or server) via its GraphQLResult, we made the choice to modify the Output values of both GraphQLQueryPager and AsyncGraphQLQueryPager such that those values are no longer tuples of (Model, UpdateSource) and are instead just Model: 1 2.

Improving loadAll

To correctly handle errors within loadAll, we first detect whether an error occurred and use that error to set isLoadingAll to false (1), which forces an exit of loadAll.

Storing the most recent result

This is important in order to know the most recent Source of an update, as well as the errors key of the most recent update. 1

Simplifying Usage

Changes to subscribe

We marked the subscribe methods of both GraphQLQueryPager and AsyncGraphQLQueryPager as deprecated: 1 2. This is because the subscribe method was already redundant: The GraphQLQueryPager and AsyncGraphQLQueryPager are Combine Publishers, and a consumer of the API can subscribe to them by calling sink.

Removal of Initializers

In conjunction with the changes related to errors, we've removed various initializers that were no longer valid given the change in return types. We considered removal of all convenience initializers and initializers that contain a transform function, but found them useful in specific applications. For example, some consumers of the library must have an output type that is not constrained to any underlying Query, as the underlying Query may be different. For those consumers, the transform: initializers remain. For all others, however, it's highly recommended that the standard transform-less initializer is used, as it retains information about the most recent value, errors within, and so on.

New computed properties

Some users have made note that they commonly employ a bit of code like this one:

let allPages = previousPages + [initialPage] + nextPages

We've added a computed property that should automatically do that for you if the pages are of the same type, as well as a computed property that surfaces all data.

Similarly, we've added a computed property that surfaces all errors from all pages.

Other Changes

We've removed conformance to Equatable on GraphQLQueryPager, as it doesn't make much sense – we were previously comparing the equatability of the current values – but not of the underlying data or state of the pager. 1 2

We've renamed the fetch case of UpdateSource to server, to be more in line with GraphQLResult.Source. 1.

Impact

The impact of this PR is:

• Users are now aware of request-level errors when using GraphQLQueryPager
• loadAll can no longer short-circuit.
• The caller API is simplifying and maturing, with fewer ways to achieve the same results. This also means a net deletion of code within the library, and less surface area to support.

[netlify]

👷 Deploy request for apollo-ios-docc pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	66a0ece

[netlify]

👷 Deploy request for eclectic-pie-88a2ba pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	66a0ece

[AnthonyMDev]

I know you’re still working on getting this PR passing CI. But I’m doing some code review now and I’m having a hard time understanding some of the changes made here. Adding this PaginationResult feels like it adds a lot of complexity, both internally and in the consumption of the API in calling code.

---

This also seems to be concatenating all errors from individual pages into one [errors] array on the PaginationResult. That is not ideal, as it will be difficult to understand which errors came from which requests and trace them back using the error path and location.

---

As is often the case, this is becoming a lot more involved than was originally anticipated to solve the issues in apollographql/apollo-ios#3413. I'm wondering if there is a different approach to addressing that without making such sweeping changes.

I'm not sure if this is the right approach either, but would it be possible to keep the transform APIs, but change them to expose the errors for each query? That could allow users to determine if they want to ignore and suppress those errors or expose them however they see fit. This could mean we are just pushing off the difficulty of dealing with per-page errors to the users, rather than handling them ourselves though...

[Iron-Ham]

cc: @AnthonyMDev I'll push changes with the swap to using GraphQLResult within PaginationOutput following this PR Did it here

[Iron-Ham]

cc: @AnthonyMDev would it be helpful to split this into a few smaller PRs?

[PYRTUS1997]

super