[ApolloPagination] Update variable mapping logic to use a Set instead of an Array #260
Conversation
👷 Deploy request for eclectic-pie-88a2ba pending review.Visit the deploys page to approve it
|
Iron-Ham
changed the title
🤣 Amazing |
There was a problem hiding this comment.
This looks good. Only question I have is this:
nextPageVarMap is currently an OrderedDictionary<AnyHashable, PaginatedQuery.Data>. But the keys should be these sets of variables. And the line in this PR is the only place we are adding data to that variable.
Shouldn't we change this property to the type OrderedDictionary<Set<JSONValue>, PaginatedQuery.Data>. This would prevent us from making any mistakes with this in the future (forgetting to wrap variables in a Set). Is there anything that needs this to be AnyHashable?
If so, we also should change previousPageVarMap.
|
@AnthonyMDev That sounds right – Updating this PR. |
7182da31 [ApolloPagination] Update variable mapping logic to use a Set instead of an Array (#260) git-subtree-dir: apollo-ios-pagination git-subtree-split: 7182da3193a3263e3522e7a0f04d670ffc081625
…riable mapping logic to use a Set instead of an Array git-subtree-dir: apollo-ios-pagination git-subtree-mainline: 67e512a git-subtree-split: 7182da3193a3263e3522e7a0f04d670ffc081625
A special thank you to @ralph-101-dev for finding this issue in a pre-release build of
ApolloPagination.Closes apollographql/apollo-ios#3331
This pull request primarily focuses on modifying how variables are handled in
AsyncGraphQLQueryPagerCoordinator.swift. The changes involve altering the data types of variables from arrays to sets, which should eliminate the possibility of a mismatch between mis-ordered array outputs. This is especially needed, because the underlying values being pulled from are an unordered dictionary, so multiple invocations of the same function can result in a key-miss.The change to production code is a one-liner: We've updated the value of the
variablesvariable – which houses the input variables of a givenGraphQLQuery– from anArray<JSONValueto aSet<JSONValue.The tests were updated to unwrap the
AnyHashablekey-value of thenextPageVarMapproperty as aSetinstead of as anArray. Notably, the tests were masking this error, as they were previously taking theArrayoutputs, wrapping them in aSet, and then comparing the sets!