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

feat(firestore): Support array-contains, array-contains-any & in filters #2868

Merged
merged 9 commits into from Nov 24, 2019

Conversation

Salakar
Copy link
Member

@Salakar Salakar commented Nov 13, 2019

Summary

Introduce support for newly released array-contains, array-contains-any & in query filters.

Blog: https://firebase.googleblog.com/2019/11/cloud-firestore-now-supports-in-queries.html

Closes #2842.

Checklist

  • Supports Android
  • Supports iOS
  • e2e tests added or updated in packages/**/e2e
  • Flow types updated
  • Typescript types updated

Think react-native-firebase is great? Please consider supporting the project with any of the below:

@Salakar Salakar added Service: Firestore Firebase Cloud Firestore Version: >= 6 labels Nov 13, 2019
@Salakar Salakar added this to the v6.1.0 milestone Nov 13, 2019
@codecov
Copy link

codecov bot commented Nov 13, 2019

Codecov Report

Merging #2868 into master will decrease coverage by 3.13%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2868      +/-   ##
==========================================
- Coverage   88.71%   85.58%   -3.12%     
==========================================
  Files         111      111              
  Lines        3444     3453       +9     
==========================================
- Hits         3055     2955     -100     
- Misses        389      498     +109

@Salakar
Copy link
Member Author

Salakar commented Nov 13, 2019

Looks like the updated BoM is causing Vision tests to fail, might just be the tests are too specific though on checking the returned results.

@mikehardy
Copy link
Collaborator

Looks like the updated BoM is causing Vision tests to fail, might just be the tests are too specific though on checking the returned results.

It appears that while the iOS SDK appears to move along as a version-locked monorepo, if you look closely at the underlying version numbers the MLKit for instance is still alpha, so has breaking changes. In camo inside the larger SDK SemVer. I suppose the Android BoM works the same?

firebase/firebase-ios-sdk#3929 (comment)

No idea how to handle that

@Salakar
Copy link
Member Author

Salakar commented Nov 17, 2019

@mikehardy I suppose the Android BoM works the same?

Aye correct. We'll just have to go fixing internal breaking changes in the code base before release, normally isn't too bad.

@meftunca
Copy link

How long do we have to wait for the release

mikehardy
mikehardy previously approved these changes Nov 22, 2019
Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

Looks good - that's a lot of noise for the same BoM/Pod version spread out everywhere. Tough trade-off on install step facility vs the repetition - would be great if that was a variable or something though. Next thing you know the build will have 5 phases though I'm sure

@mikehardy
Copy link
Collaborator

@meftunca I'm sure it would be great if someone could test it, please do so and report back.

Here I provide a version of the PR you can apply using patch-package (which I swear by, excellent tool) -
@react-native-firebase+firestore+6.0.4.patch.gz

Just install patch-package per instructions (very quick+easy) and unzip that patch into the patches directory. It's a text file, you may visually confirm it contains the functional changes here.

You will need to override the underlying android and ios google SDK versions to do this. I demonstrate the 2 changes you need in your project here to specify the versions that will work: https://github.com/mikehardy/rnfbdemo/blob/master/make-demo-v6.sh#L28

In a few minutes you should have the PR in your project and hearing it works would be very helpful.

@Salakar I haven't figured out how to do patch-package effectively in monorepos yet. The ideal flow is "change yarn dependency in work project from regular version to git, optionally run a local build inside node_modules (typescript etc), run patch-package to generate a diff". In monorepos it's "clone repo, pull branch, run build, copy altered files into node_modules, run patch_package" which is really heavy. I haven't seen anything recommended online on how to solve it other than "gitpkg" but maybe you have an idea. Either way, I'm playing with v6 now 👀

@@ -1,733 +0,0 @@
PODS:
Copy link
Collaborator

@mikehardy mikehardy Nov 22, 2019

Choose a reason for hiding this comment

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

removing this file causes conflicts on rebase to master but it's just a git add tests/ios/Podfile.lock fix.

Copy link

@meftunca meftunca Nov 22, 2019

Choose a reason for hiding this comment

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

@mikehardy I'm sorry to see you late. You can rest assured that I'll send you feedback after I review it.

@meftunca
Copy link

meftunca commented Nov 22, 2019

@meftunca I'm sure it would be great if someone could test it, please do so and report back.

Here I provide a version of the PR you can apply using patch-package (which I swear by, excellent tool) -
@react-native-firebase+firestore+6.0.4.patch.gz

Just install patch-package per instructions (very quick+easy) and unzip that patch into the patches directory. It's a text file, you may visually confirm it contains the functional changes here.

You will need to override the underlying android and ios google SDK versions to do this. I demonstrate the 2 changes you need in your project here to specify the versions that will work: https://github.com/mikehardy/rnfbdemo/blob/master/make-demo-v6.sh#L28

In a few minutes you should have the PR in your project and hearing it works would be very helpful.

You can be sure that it works smoothly :)
Screen Shot 2019-11-23 at 12 07 06 AM
Screen Shot 2019-11-23 at 12 07 16 AM

I've been waiting for this property for a week. I couldn't write a proper description. I have currently tried it on android and has no problems with testing.

@mikehardy
Copy link
Collaborator

Fantastic! Thanks for testing it in the real world, that really helps @meftunca

@Salakar Salakar marked this pull request as ready for review November 23, 2019 17:10
…rebase into @feat/firestore/in-query

# Conflicts:
#	tests/ios/Podfile.lock
@Salakar Salakar merged commit 42e034c into master Nov 24, 2019
7patricia pushed a commit to uphold-forks/react-native-firebase that referenced this pull request Nov 29, 2019
…ertase#2868)

* feat(firestore) Add IN query support (JS/Android)
* feat(firestore) in query validation
* feat(firestore) in query ios support / tests
* docs(firestore): update reference docs to include in query support
andersondanilo pushed a commit to vixtech/react-native-firebase that referenced this pull request Nov 9, 2020
…ertase#2868)

* feat(firestore) Add IN query support (JS/Android)
* feat(firestore) in query validation
* feat(firestore) in query ios support / tests
* docs(firestore): update reference docs to include in query support
andersondanilo pushed a commit to vixtech/react-native-firebase that referenced this pull request Nov 9, 2020
…ertase#2868)

* feat(firestore) Add IN query support (JS/Android)
* feat(firestore) in query validation
* feat(firestore) in query ios support / tests
* docs(firestore): update reference docs to include in query support
@mikehardy mikehardy deleted the @feat/firestore/in-query branch December 17, 2020 20:59
hmhm2292 pushed a commit to hmhm2292/react-native-firebase that referenced this pull request Jul 13, 2021
…ertase#2868)

* feat(firestore) Add IN query support (JS/Android)
* feat(firestore) in query validation
* feat(firestore) in query ios support / tests
* docs(firestore): update reference docs to include in query support
androidIsForVivek pushed a commit to androidIsForVivek/react-native-firebase that referenced this pull request Aug 9, 2021
…ertase#2868)

* feat(firestore) Add IN query support (JS/Android)
* feat(firestore) in query validation
* feat(firestore) in query ios support / tests
* docs(firestore): update reference docs to include in query support
androidIsForVivek pushed a commit to androidIsForVivek/react-native-firebase that referenced this pull request Sep 15, 2021
…ertase#2868)

* feat(firestore) Add IN query support (JS/Android)
* feat(firestore) in query validation
* feat(firestore) in query ios support / tests
* docs(firestore): update reference docs to include in query support
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Service: Firestore Firebase Cloud Firestore Version: >= 6
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement the in/array-contains-any queries
4 participants