-
Notifications
You must be signed in to change notification settings - Fork 57
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
JSONConverter helper #380
JSONConverter helper #380
Conversation
✅ Deploy Preview for apollo-ios-docc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for eclectic-pie-88a2ba canceled.
|
/// Converts a ``SelectionSet`` into a basic JSON dictionary for use. | ||
/// | ||
/// - Returns: A `[String: Any]` JSON dictionary representing the ``SelectionSet``. | ||
public static func convert(_ selectionSet: some SelectionSet) -> [String: Any] { |
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.
as you might have guessed from my PR, I'm not a fan of relying on the compiler to select the right overload, but that's my personal preference. The changes in this PR still work fine for my use case. Thanks for the help!
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.
LGTM! Do we have a test for the GraphQLError
conversion anywhere already?
79a3e17
to
4a743b3
Compare
Yes, it's at GraphQLResultTests.swift:69 |
b52a8610 JSONConverter helper (#380) git-subtree-dir: apollo-ios git-subtree-split: b52a86100fd041481a9f191f4a1ab007069a41a1
git-subtree-dir: apollo-ios git-subtree-mainline: 4ef0f28 git-subtree-split: b52a86100fd041481a9f191f4a1ab007069a41a1
This is an alternative API for the changes in #377. It also cleans up the unit tests so we don't need to import
StarWarsAPI
in more places (hoping we can get rid of it completely eventually).@TizianoCoroneo @calvincestari @BobaFetters I think this is an improvement on the API in the other PR. Let me know what you think.