Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Handle coercion of AnyHashable #61

Merged
merged 2 commits into from
Oct 5, 2023
Merged

Handle coercion of AnyHashable #61

merged 2 commits into from
Oct 5, 2023

Conversation

AnthonyMDev
Copy link
Contributor

In iOS versions 14.4 and lower, AnyHashable coercion does not work. On these platforms, we need to do some additional unwrapping and casting of the values to avoid crashes and other run time bugs.

In addition to doing some more explicit handling of the null values, this PR changes the check for the coercion from
#if swift (>=5.4) to if #available(iOS 14.5, *). It turns out that this is not particularly related to the swift version, but to the iOS version being run.

This PR has been tested locally on iOS 14.4, 14.5, and 17.0.

@AnthonyMDev AnthonyMDev mentioned this pull request Oct 5, 2023
Closed
BobaFetters
BobaFetters reviewed Oct 5, 2023
View reviewed changes
apollo-ios/Sources/ApolloAPI/DataDict.swift Outdated
@@ -179,11 +216,11 @@ extension Array: SelectionSetEntityValue where Element: SelectionSetEntityValue
fatalError("\(Self.self) expected list of data for entity.")
}
self = data.map {
#if swift(>=5.4)
if DataDict._AnyHashableCanBeCoerced {
Element.init(_fieldData:$0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like these lines need return statements now since its no longer technically a single line of code because of the #if check

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, thanks. just fixed that. Oddly it was still compiling and running fine on my machine with out them. I have no idea how.

BobaFetters
BobaFetters approved these changes Oct 5, 2023
View reviewed changes
Copy link
Member

@BobaFetters BobaFetters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Nice work!

@AnthonyMDev AnthonyMDev merged commit ecacc66 into main Oct 5, 2023
11 checks passed
@AnthonyMDev AnthonyMDev deleted the ios-14-support-fix branch October 5, 2023 20:01
BobaFetters pushed a commit that referenced this pull request Oct 5, 2023
@AnthonyMDev
Handle coercion of AnyHashable (#61)
7589d5b
BobaFetters pushed a commit to apollographql/apollo-ios that referenced this pull request Oct 5, 2023
@AnthonyMDev
Handle coercion of AnyHashable (apollographql/apollo-ios-dev#61)
7349dd8
BobaFetters pushed a commit that referenced this pull request Oct 5, 2023
Squashed 'apollo-ios/' changes from f304227..7349dd8
5697490 
7349dd8 Handle coercion of AnyHashable  (#61)

git-subtree-dir: apollo-ios
git-subtree-split: 7349dd8
BobaFetters pushed a commit that referenced this pull request Oct 5, 2023
split: apollo-ios - PR #61 - Handle coercion of AnyHashable
00e39ec 
git-subtree-dir: apollo-ios
git-subtree-mainline: 7589d5b
git-subtree-split: 7349dd8
@matijakregarGH
Copy link

@AnthonyMDev
Thanks for the quick response, however an(other) issue still exists (Apollo v 1.6.0, iOS v 14.4.1).
I'm now getting a fatal error in DataDict file (line 182) in the RootSelectionSet extension guard statement.
Fatal error: ... expected DataDict for entity, got Optional<AnyHashable>.

Called from extension Optional: SelectionSetEntityValue where Wrapped: SelectionSetEntityValue (DataDict line 203)
Called from @inlinable public subscript<T: SelectionSetEntityValue>(_ key: String) -> T (DataDict line 78)

Please let me know how I can help to get this resolved ...

Best,
Matija

@AnthonyMDev
Copy link
Contributor Author

Thank you for the detailed report! I'll look into this today.

@AnthonyMDev
Copy link
Contributor Author

I had forgotten the ! in front of one of the conditions for checking if we were on iOS 14.5+. Should be if NOT on iOS 14.5+.
PR is up to fix this now. #68

We'll get a patch out ASAP

@VMLe VMLe mentioned this pull request Oct 23, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BobaFetters BobaFetters BobaFetters approved these changes

@calvincestari calvincestari Awaiting requested review from calvincestari

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AnthonyMDev @matijakregarGH @BobaFetters