BUG: getAggregateFromServer does not consider any where clauses attached to its query

[joaqo]

Issue

As requested by @mikehardy I created this issue that originated from this comment on PR #8115.

While trying (unsuccessfully) to use the new sum aggregated query, I found out that the getAggregateFromServer function does not consider where clauses attached to it, at least when used with collection groups. Here is a MRE.

import firestore from "@react-native-firebase/firestore"
db = firestore()

const ref = db.collectionGroup("orders").where("sellerId", "==", "123abc").where("status", "==", "arrived")

const correct = await ref.count().get()
console.log("Count correct", correct.data().count)  // Correctly counts filtered orders

const incorrect = await getAggregateFromServer(ref, { countCollection: count() })
console.log("Count incorrect", incorrect.data().countCollection)  // Counts ALL orders, for all sellers, which is incorrect!

This issue is particularly worrisome as it completely disallows the usage of the new sum and average functions, as they don't yet have support for method chaining that count does, so using them through getAggregateFromServer is the only way to use them!

Using RNFB 21.7.1.

---

Project Files

Javascript

Click To Expand

package.json:

{
  "name": "app",
  "main": "node_modules/expo/AppEntry.js",
  "version": "1.0.0",
  "scripts": {
    "start": "expo start",
    "android": "tsc --noemit && expo run:android",
    "ios": "tsc --noemit && expo run:ios",
    "web": "expo start --web",
    "test": "jest --watchAll",
    "lint": "eslint .",
    "postinstall": "patch-package",
    "release": "npm run lint && npx expo prebuild -p ios --clean && open ios/app.xcworkspace/ && eas build -p android"
  },
  "jest": {
    "preset": "jest-expo"
  },
  "dependencies": {
    "@babel/runtime": "^7.24.0",
    "@expo/vector-icons": "^14.0.3",
    "@gorhom/bottom-sheet": "^4.6.4",
    "@likashefqet/react-native-image-zoom": "3.00",
    "@react-native-clipboard/clipboard": "^1.14.1",
    "@react-native-community/datetimepicker": "8.0.1",
    "@react-native-firebase/analytics": "^21.7.1",
    "@react-native-firebase/app": "^21.7.1",
    "@react-native-firebase/auth": "^21.7.1",
    "@react-native-firebase/crashlytics": "^21.7.1",
    "@react-native-firebase/firestore": "^21.7.1",
    "@react-native-firebase/functions": "^21.7.1",
    "@react-native-firebase/messaging": "^21.7.1",
    "@react-native-firebase/storage": "^21.7.1",
    "@react-native-menu/menu": "1.0.2",
    "@react-native-picker/picker": "2.7.5",
    "@react-navigation/bottom-tabs": "^6.5.20",
    "@react-navigation/material-top-tabs": "^6.6.13",
    "@react-navigation/native": "^6.0.2",
    "@react-navigation/native-stack": "^6.9.26",
    "@shopify/flash-list": "1.6.4",
    "@sinonjs/text-encoding": "^0.7.2",
    "@tmcw/togeojson": "^5.8.1",
    "@types/jsrsasign": "^10.5.14",
    "@types/pem": "^1.14.4",
    "@xmldom/xmldom": "^0.8.10",
    "date-fns": "^3.6.0",
    "date-fns-tz": "^3.2.0",
    "expo": "^51.0.30",
    "expo-application": "~5.9.1",
    "expo-av": "~14.0.7",
    "expo-blur": "~13.0.2",
    "expo-build-properties": "~0.12.5",
    "expo-device": "~6.0.2",
    "expo-document-picker": "~12.0.2",
    "expo-file-system": "~17.0.1",
    "expo-image": "~1.12.14",
    "expo-image-manipulator": "~12.0.5",
    "expo-image-picker": "~15.0.7",
    "expo-linking": "~6.3.1",
    "expo-localization": "~15.0.3",
    "expo-location": "~17.0.1",
    "expo-splash-screen": "~0.27.5",
    "expo-status-bar": "~1.12.1",
    "expo-system-ui": "~3.0.7",
    "expo-web-browser": "~13.0.3",
    "geolib": "^3.3.4",
    "immer": "^10.0.3",
    "lottie-react-native": "6.7.0",
    "patch-package": "^8.0.0",
    "pem": "^1.14.8",
    "posthog-react-native": "^3.3.6",
    "posthog-react-native-session-replay": "^0.1.6",
    "react": "18.2.0",
    "react-dom": "18.2.0",
    "react-instantsearch-core": "^7.7.0",
    "react-native": "0.74.5",
    "react-native-dialog": "^9.3.0",
    "react-native-draggable-flatlist": "^4.0.1",
    "react-native-draglist": "^3.5.1",
    "react-native-gesture-handler": "~2.16.1",
    "react-native-google-places-autocomplete": "^2.5.6",
    "react-native-haptic-feedback": "^2.3.2",
    "react-native-ios-utilities": "^4.5.2",
    "react-native-linear-gradient": "^2.8.3",
    "react-native-maps": "1.14.0",
    "react-native-mmkv": "^2.12.2",
    "react-native-notifier": "^2.0.0",
    "react-native-pager-view": "6.3.0",
    "react-native-picker-select": "^9.0.1",
    "react-native-progress": "^5.0.1",
    "react-native-prompt-android": "^1.1.0",
    "react-native-quick-crypto": "0.7.3",
    "react-native-reanimated": "~3.10.1",
    "react-native-reanimated-carousel": "^3.5.1",
    "react-native-safe-area-context": "4.10.5",
    "react-native-screens": "3.31.1",
    "react-native-snackbar": "^2.6.2",
    "react-native-svg": "15.2.0",
    "react-native-tab-view": "^3.5.2",
    "react-native-web": "~0.19.6",
    "react-native-webview": "13.8.6",
    "safe-stable-stringify": "^2.4.3",
    "togeojson": "^0.16.0",
    "typesense": "^1.7.2",
    "typesense-instantsearch-adapter": "^2.8.0",
    "zeego": "^2.0.4",
    "zustand": "^4.5.0"
  },
  "devDependencies": {
    "@babel/core": "^7.20.0",
    "@types/react": "~18.2.45",
    "eslint": "8",
    "eslint-config-expo": "^7.1.2",
    "husky": "^9.1.5",
    "jest": "^29.2.1",
    "jest-expo": "~51.0.4",
    "lint-staged": "^15.2.2",
    "prettier": "3.2.5",
    "react-test-renderer": "18.2.0",
    "ts-node": "^10.9.2",
    "typescript": "~5.3.3",
    "uri-scheme": "^1.2.2"
  },
  "overrides": {
    "@react-native-clipboard/clipboard": {
      "react-native": "~0.74.0",
      "react-native-windows": "~0.74.0"
    }
  },
  "private": true,
  "lint-staged": {
    "*.{js,jsx,ts,tsx,css,md}": "prettier --write"
  }
}

firebase.json for react-native-firebase v6:

{
  "react-native": {
    "crashlytics_debug_enabled": false
  }
}

iOS

Click To Expand

ios/Podfile:

• I'm not using Pods
• I'm using Pods and my Podfile looks like:

require File.join(File.dirname(`node --print "require.resolve('expo/package.json')"`), "scripts/autolinking")
require File.join(File.dirname(`node --print "require.resolve('react-native/package.json')"`), "scripts/react_native_pods")

require 'json'
podfile_properties = JSON.parse(File.read(File.join(__dir__, 'Podfile.properties.json'))) rescue {}

ENV['RCT_NEW_ARCH_ENABLED'] = podfile_properties['newArchEnabled'] == 'true' ? '1' : '0'
ENV['EX_DEV_CLIENT_NETWORK_INSPECTOR'] = podfile_properties['EX_DEV_CLIENT_NETWORK_INSPECTOR']

use_autolinking_method_symbol = ('use' + '_native' + '_modules!').to_sym
origin_autolinking_method = self.method(use_autolinking_method_symbol)
self.define_singleton_method(use_autolinking_method_symbol) do |*args|
  if ENV['EXPO_UNSTABLE_CORE_AUTOLINKING'] == '1'
    Pod::UI.puts('Using expo-modules-autolinking as core autolinking source'.green)
    config_command = [
      'node',
      '--no-warnings',
      '--eval',
      'require(require.resolve(\'expo-modules-autolinking\', { paths: [require.resolve(\'expo/package.json\')] }))(process.argv.slice(1))',
      'react-native-config',
      '--json',
      '--platform',
      'ios'
    ]
    origin_autolinking_method.call(config_command)
  else
    origin_autolinking_method.call()
  end
end

platform :ios, podfile_properties['ios.deploymentTarget'] || '13.4'
install! 'cocoapods',
  :deterministic_uuids => false

prepare_react_native_project!

target 'R E D A C T E D' do
  use_expo_modules!
  config = use_native_modules!

  use_frameworks! :linkage => podfile_properties['ios.useFrameworks'].to_sym if podfile_properties['ios.useFrameworks']
  use_frameworks! :linkage => ENV['USE_FRAMEWORKS'].to_sym if ENV['USE_FRAMEWORKS']

  use_react_native!(
    :path => config[:reactNativePath],
    :hermes_enabled => podfile_properties['expo.jsEngine'] == nil || podfile_properties['expo.jsEngine'] == 'hermes',
    # An absolute path to your application root.
    :app_path => "#{Pod::Config.instance.installation_root}/..",
    :privacy_file_aggregation_enabled => podfile_properties['apple.privacyManifestAggregationEnabled'] != 'false',
  )

  post_install do |installer|
    react_native_post_install(
      installer,
      config[:reactNativePath],
      :mac_catalyst_enabled => false,
      :ccache_enabled => podfile_properties['apple.ccacheEnabled'] == 'true',
    )

    # This is necessary for Xcode 14, because it signs resource bundles by default
    # when building for devices.
    installer.target_installation_results.pod_target_installation_results
      .each do |pod_name, target_installation_result|
      target_installation_result.resource_bundle_targets.each do |resource_bundle_target|
        resource_bundle_target.build_configurations.each do |config|
          config.build_settings['CODE_SIGNING_ALLOWED'] = 'NO'
        end
      end
    end
  end

  post_integrate do |installer|
    begin
      expo_patch_react_imports!(installer)
    rescue => e
      Pod::UI.warn e
    end
  end
end

AppDelegate.m:

// N/A

---

Android

Click To Expand

Have you converted to AndroidX?

• my application is an AndroidX application?
• I am using android/gradle.settings jetifier=true for Android compatibility?
• I am using the NPM package jetifier for react-native compatibility?

android/build.gradle:

// N/A

android/app/build.gradle:

// N/A

android/settings.gradle:

// N/A

MainApplication.java:

// N/A

AndroidManifest.xml:

<!-- N/A -->

---

Environment

Click To Expand

• Platform that you're experiencing the issue on:
  • iOS
  • Android
  • [x ] iOS but have not tested behavior on Android
  • Android but have not tested behavior on iOS
  • Both
• react-native-firebase version you're using that has this issue:
  • 21.7.1
• Firebase module(s) you're using that has the issue:
  • firestore
• Are you using TypeScript?
  • Yes ~5.3.3

---

• 👉 Check out React Native Firebase and Invertase on Twitter for updates on the library.

[tomas1808]

Same problem here, can't use sum or average

[SelaseKay]

Hi @joaqo, thanks for the report. I've been able to reproduce this issue with the snippet you shared. This issue will be further investigated.

[mikehardy]

@joaqo - looks like @SelaseKay has a fix queued up for this in #8259 ( 💪 !) - but I want to note that the fix is in the iOS native code only. I reviewed the original PR this bug spawned from and it appears to me that the android native code from the original PR was already applying the where (or any other filters) to the aggregate query correctly so I don't think it needed repair.

But just to be sure: can you confirm that you saw the erroneous non-filtered behavior on iOS, but that it was working correctly already on android?

[mikehardy]

With an e2e test probing this and showing it works on both platforms I'm going for the default answer "it's fine on android" and I've approved the PR, personally. Hopefully we can get this fix merged and released shortly

[joaqo]

Sorry, was away from computer for a few days. Can confirm that version 21.7.2 fixes the issue on iOS and Android, though Android didn't have the issue in the first place I believe.

Thank you so much for this 😊

PD: I know this is off-topic but version 21.7.2 adds hundreds of warnings like the following to my app:

This v8 method is deprecated and will be removed in the next major release as part of move to match Firebase Web modular v9 SDK API. Please use instead.

I am not using the web sdk in any way shape or form. Regrettably this makes this version useless for me. I was on version 21.7.1 prior to this. Seems unusual that a small patch update like this would explode my app in this way.

[mikehardy]

Thank you so much for this 😊

Glad to hear it!

I am not using the web sdk in any way shape or form. Regrettably this makes this version useless for me. I was on version 21.7.1 prior to this. Seems unusual that a small patch update like this would explode my app in this way.

I do understand, but will ask for understanding in return:

1. We are obligated to keep pace with google's firebase-js-sdk as our fundamental API reference
2. firebase-js-sdk is about to remove the v8 / non-modular APIs and we must follow them
3. we are obligated to go through a deprecation cycle prior to doing so, and deprecation notices indeed are not a semver-major or minor, they are a warning prior to a semver-major

Developers must (I emphasize must) migrate to the new modular-style APIs so I beg you instead to actually do the transition

All of the packages will receive those deprecation notices shortly, we've only done a few packages so far.

I will note that I originally feared it was going to be awful in my personal / work projects and ... it wasn't at all. The API transition is literally mechanical. Every single API has a corresponding API that behaves exactly the same, it was just import changes and re-arranging the call. In the end it was the work of an hour or two with no issues

[joaqo]

Oh! I completely misunderstood the warning message then. I thought it was some sort of weird dependency that you had on the official firebase web sdk that was misbehaving.

What you're telling me is that you're purposely moving your own api to more closely match the official firebase web sdk's modular api? That actually sounds great!

We are going to be adding a web version to our app, and we were dreading having to create an abstraction over RNFB and the web sdk so that our components can consume a single api and therefore maximize code share between web and mobile. It seems that this new api is going to make that much easier, or unnecessary even.

I'll get moving towards the new api as soon as possible. 😄

Thanks again for your hard work!

PD: This seems like quite a big change on your end, have you communicated this to your userbase? It took me completely off guard at least. Also the current warning messages is not so clear, it talks about v8 methods vs v9 methods while RNFB is currently on v21. Thats the reason I originally thought it was a firebase web sdk dependency bug instead of a RNFB api change.

[mikehardy]

What you're telling me is that you're purposely moving your own api to more closely match the official firebase web sdk's modular api? That actually sounds great!

Believe it or not we already have excellent fidelity with firebase-js-sdk v8 and v9+ APIs, it's just that we've been around so long that everyone still just uses the old v8 APIs. We are supposed to be basically drop-in compatible. firebase-js-sdk is only just now actually getting rid of the v8 APIs though and we have to follow suit

PD: This seems like quite a big change on your end, have you communicated this to your userbase? It took me completely off guard at least. Also the current warning messages is not so clear, it talks about v8 methods vs v9 methods while RNFB is currently on v21. Thats the reason I originally thought it was a firebase web sdk dependency bug instead of a RNFB api change.

Working on it - your feedback on the messaging is useful, I'll pass it along that the versioning reference is not great

[joaqo]

Awesome, thanks!