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.

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

expo-contacts: fix getting containers without a predicate (and crash) #6016

Merged
merged 1 commit into from Oct 19, 2019

Conversation

@tasn
Copy link
Contributor

tasn commented Oct 17, 2019

The predicate was not initialised in all cases, which meant that an invalid
predicate passed from javascript would make it crash.

Additionally, according to the Apple's docs[1], passing nil as the predicate
is fine and just means no filter. At the moment there's no way to get the
whole list of containers without filtering. This patch fixes that.

1: https://developer.apple.com/documentation/contacts/cncontactstore/1403238-containersmatchingpredicate?language=objc

Why

Fixes a crash + makes it possible to query all of the containers.

How

I want to be able to get all of the containers on the device. In order to do that, you have to be able to pass nil. This fixes it.

Test Plan

Call Contacts.getContainersAsync(nil)

The predicate was not initialised in all cases, which meant that an invalid
predicate passed from javascript would make it crash.

Additionally, according to the Apple's docs[1], passing nil as the predicate
is fine and just means no filter. At the moment there's no way to get the
whole list of containers without filtering. This patch fixes that.

1: https://developer.apple.com/documentation/contacts/cncontactstore/1403238-containersmatchingpredicate?language=objc
@tasn tasn requested a review from EvanBacon as a code owner Oct 17, 2019
@tasn

This comment has been minimized.

Copy link
Contributor Author

tasn commented Oct 17, 2019

The build failures don't seem related to me. It's a very simple fix. Please review anyway. :)

@tasn

This comment has been minimized.

Copy link
Contributor Author

tasn commented Oct 17, 2019

Also, this should probably be backported to previous expo versions, though no idea how to do that.

Copy link
Contributor

EvanBacon left a comment

LGTM

@EvanBacon EvanBacon merged commit acbacf4 into expo:master Oct 19, 2019
1 of 3 checks passed
1 of 3 checks passed
client Workflow: client
Details
sdk Workflow: sdk
Details
docs Workflow: docs
Details
@tasn

This comment has been minimized.

Copy link
Contributor Author

tasn commented Oct 19, 2019

How can we backport it to previous expo versions so we'll get it in 35.0.1 and not have to wait until 36?

Thanks!

tasn added a commit to tasn/expo that referenced this pull request Nov 14, 2019
…expo#6016)

The predicate was not initialised in all cases, which meant that an invalid
predicate passed from javascript would make it crash.

Additionally, according to the Apple's docs[1], passing nil as the predicate
is fine and just means no filter. At the moment there's no way to get the
whole list of containers without filtering. This patch fixes that.

1: https://developer.apple.com/documentation/contacts/cncontactstore/1403238-containersmatchingpredicate?language=objc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.