-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove stripping of null values from Selection Set models #25
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
BobaFetters
approved these changes
Aug 31, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM, nice work!
BobaFetters
pushed a commit
that referenced
this pull request
Aug 31, 2023
BobaFetters
pushed a commit
to apollographql/apollo-ios
that referenced
this pull request
Aug 31, 2023
BobaFetters
added a commit
that referenced
this pull request
Aug 31, 2023
AnthonyMDev
added a commit
to apollographql/apollo-ios
that referenced
this pull request
Sep 5, 2023
BobaFetters
pushed a commit
that referenced
this pull request
Oct 6, 2023
c1ad1ad Merge pull request #3250 from apollographql/project-breakup b743d6b Merge branch 'main' into project-breakup 6dacd0b docs: remove preview status from persisted queries (#3241) 42802f5 Release 1.5.2 (#3245) 56e94ba Adding codegen deprecation message (#3243) c1ff0fc Release 1.5.1 (#3244) d1b3da6 update false positive secret detection allowlist (#3239) 72dbf4c Update ROADMAP.md 453738a only output operation manifest once (#3225) f79e659 docs: Updates sample code for Advanced Network Configuration documentation (#3223) 6e3bc47 chore: Changed to use isEmpty instead of count == 0 (#3217) e76c9e7 fix: Fixed reading of the `itemsToGenerate` property for the operation manifest (#3215) 76a7619 2023-09-07 Roadmap update (#3214) 7607b8b fix: Updated the target codegen configuration (#3212) 5f93599 feature: final class definitions for graphql codegen (#3189) 0773964 Release/1.5 (#3211) 18108c3 Remove stripping of null values from Selection Set models (#25) 888461d Adds RequestContext to the RequestChain/Interceptor pipeline (#3198) e10dc84 Updating docs for `itemsToGenerate` (#3200) 6d253e2 docs: Adding full codegen config example (#3193) 7b02cb0 docs: Fix field conversion strategy docs (#3192) 8c7f1cc Update persisted queries docs (#3160) d9e83f2 Roadmap update 2023-08-10 f94d367 Release: 1.4.0 (#3184) c608c5e feature: Add support for field casing strategy to code gen (#3171) e21116a Updating Persisted Query Configuration (#3175) 4147f78 refactor: Fragment types in IR (#3174) 1e0f8dd docs: Clarify `other` schema module type (#3170) c3f6951 Release/1 3 3 (#3169) 2289679 fix: Removing newline characters from operation id hashes (#3163) 5065f51 Fix merged selection set name ambiguity at definition root (#3168) aaeed59 Roadmap update 2023-07-27 fa28628 Also look for Package.resolved in .xcworkspace when performing version check (#3048) a47a8af feature: Adding CLI command for Operation Manifest (#3152) e95cec8 fix: Updated mock function descriptor handling (#3150) 1d0badc fix: Fragment definitions support escaped characters (#3135) c1d9a22 Update pull-request-template-release.md 97ad4f5 Release: 1.3.2 (#3133) d840428 fix: Update graphql-js error handling (#3132) 874c7ff Deprecating queryStringLiteralFormat (#3129) 5ddf770 Issue 2426 improve response code error api (#3123) b2a3054 chore: Delete unused imports and declarations (#3100) 7515d46 Operation Manifest Generation Updates (#3128) c338ee9 Throw error when an invalid key is present in a `JSON` Apollo configuration (#3125) 9961d2b fix: fixes two issues in the generation of mocks. (#3120) efdae87 Update CODEOWNERS.md (#3127) df4648b Fix documentation bug 93b6578 release: 1.3.1 (#3117) ec05b00 Persisted Queries [1/x] Define new persisted queries config (#3095) 03baa31 fix: Updating test mock set method definitions (#3089) 0531744 docs: Update subscriptions for multipart over HTTP (#2926) 4cc0751 Fixes ApolloInterceptor API documentation 3111cb8 Adds documentation to ApolloInterceptor d2bbeb0 Update CHANGELOG.md 84d797e release: `1.3.0` (#3085) 917a48d Merge branch 'release/1.3' 79fec42 ci: Enable manual clone from forked PRs (#3080) 2c2daab fix: Exclude script from target source (#3083) a3a1eae Release 1.2.2 (#3077) f80aadb feat: Enable SOCKS proxy support (#3076) 077430a Fix parsing of input objects as default values for input params (#3074) ff49bec Fix merged source name generation when shared root is operation root (#3073) 2ba71de Updated 1.3 migration guide to include Reserved Keyword change (#3072) 9f48589 Merge branch 'main' into release/1.3 1ec00c8 fix: Remove `managedSelf` from `InterceptorRequestChain` (#3070) f64927c Fix: Added missing fulfilledFragments to generated initializers (#3067) de57ae4 docs: Updates `1.2.0` migration guide for access modifiers default (#3068) e31ee4a fix: Adding type name suffixing for reserved keywords (#3058) 6510505 docs: Updates cache policy documentation (#3062) 759a665 Release: 1.2.1 (#3059) c5195d6 2023-06-01 Roadmap Update (#3055) 4771541 Fix RootEntityType and FulfilledFragment name generation (#3045) d757f12 ci: Update tooling versions (#3052) git-subtree-dir: apollo-ios git-subtree-split: c1ad1ad
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.
This PR removes the stripping of null values from the selection set models.
This fixes apollographql/apollo-ios#3092. Because data returned from a network request was having null values stripped, when the returned models were then manually written to the cache, the null values were not written. This results in a
missingValue
error when you try to read those objects from the cache later.This situation could occur if you fetched from the network with a cache policy of
.fetchIgnoringCacheCompletely
and then wrote some of the data to the cache manually. It also occurs in the common case of a fetched object be appended to another list of objects in a cache mutation.Null Value Representation
This bug fix revealed another related bug. We previously used
NSNull
to represent null values in aSelectionSet
, then in order to work for Swift 5.3 and below, we needed to change this. We changed it toOptional<AnyHashable>.none
, which actually was justnil
. Thesenil
values were being stripped by the executor as well.In order to handle null values in the selection set appropriately, we need to store them as a concrete value. This is now represented as
AnyHashable(Optional<AnyHashable>.none)
, which wraps anil
value inside of anAnyHashable
struct. This is a concrete value instead of justnil
, which provides the desired behavior.To make the code clearer and more expressive, I've created a
DataDict.NullValue
static property that contains this value.