fix: legacyResponse property
#456
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for apollo-ios-docc canceled.
|
✅ Deploy Preview for eclectic-pie-88a2ba canceled.
|
calvincestari
commented
Aug 7, 2024
| @@ -101,14 +101,15 @@ public struct IncrementalJSONResponseParsingInterceptor: ApolloInterceptor { | |||
| } | |||
| } | |||
|
|
|||
| createdResponse._legacyResponse = nil | |||
There was a problem hiding this comment.
I think this is the correct action to take here. There is nothing to set it to other than nil because this code path is for incremental responses. I think leaving the initial (partial) response in _legacyResponse is also incorrect because it doesn't match the result being returned. The 'legacy' part of the response logic could also be argued to mean pre-defer-support in which case nil again seems the correct choice.
BobaFetters
pushed a commit
that referenced
this pull request
Aug 8, 2024
BobaFetters
pushed a commit
to apollographql/apollo-ios
that referenced
this pull request
Aug 8, 2024
BobaFetters
pushed a commit
that referenced
this pull request
Aug 8, 2024
2ab4500e fix: `legacyResponse` property (#456) git-subtree-dir: apollo-ios git-subtree-split: 2ab4500ebc61a11a7c5344f1d7a3f4999aea4eab
BobaFetters
pushed a commit
that referenced
this pull request
Aug 8, 2024
git-subtree-dir: apollo-ios git-subtree-mainline: ae99271 git-subtree-split: 2ab4500ebc61a11a7c5344f1d7a3f4999aea4eab
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When the
legacyResponseproperty ofHTTPResponsewas deprecated setting the value was also removed; this was incorrect as it created a hidden breaking change for interceptors that might have been using the value.The value cannot simply be set though, as it was before, because of Swift's inability to disable the deprecation warning. Instead there is now an internal property that stores the legacy response value and
legacyResponsebecomes a computed property that simply forwards the new internal property value.The
cacheRecordsproperty that was added to eliminate the need to re-parse the legacy response has also been made public for interceptors that need cache records and do not want to have to parse the response a second time.