search

invertase / react-native-firebase

🔥 A well-tested feature-rich modular Firebase implementation for React Native. Supports both iOS & Android platforms for all Firebase services.
https://rnfirebase.io
Other
11.68k stars 2.21k forks source link

[🐛] Analytics opt-in key in firebase.json not carried into iOS plist file #4473

Closed pehagg closed 3 years ago

pehagg commented 3 years ago

Issue

We need to use an opt-in strategy for our analytics collection due to legal reasons. However, based on what I'm seeing in both production and Firebase's DebugView, it looks like the analytics_auto_collection_enabled setting in firebase.json has no effect. Events are being sent until the user turns off collection in the app settings UI. I couldn't find any reference in the codebase that would be reading this setting, the only references are in docs and the type definition, hence I suspect the actual behaviour differs from what the documentation suggests. I have tested this only on iOS for now but I'd assume Android sports the same issue, as we're seeing way too many events in production when considering this should be an opt-in feature.

We have set analytics_auto_collection_enabled to false in firebase.json and in the settings UI we use analytics().setAnalyticsCollectionEnabled to toggle collection on/off.

Project Files

Javascript

Click To Expand

#### `package.json`: ```json { ... "dependencies": { ... "@react-native-firebase/analytics": "7.4.2", "@react-native-firebase/app": "8.3.1", "@react-native-firebase/auth": "9.0.0", "@react-native-firebase/crashlytics": "8.3.2", "@react-native-firebase/dynamic-links": "7.4.2", "@react-native-firebase/firestore": "7.5.3", "@react-native-firebase/functions": "7.3.2", "@react-native-firebase/remote-config": "8.1.4", ... }, ... } ``` #### `firebase.json` for react-native-firebase v6: ```json { "react-native": { "crashlytics_auto_collection_enabled": true, "crashlytics_disable_auto_disabler": true, "analytics_auto_collection_enabled": false } } ```

iOS

Click To Expand

#### `ios/Podfile`: - [ ] I'm not using Pods - [x] I'm using Pods and my Podfile looks like: ```ruby require_relative '../node_modules/react-native-unimodules/cocoapods' require_relative '../node_modules/react-native/scripts/react_native_pods' require_relative '../node_modules/@react-native-community/cli-platform-ios/native_modules' platform :ios, '10.0' # $FirebaseSDKVersion = '6.31.0' target 'Compensate' do config = use_native_modules! # Automatically detect installed unimodules use_unimodules! use_react_native!(:path => config["reactNativePath"]) end ``` #### `AppDelegate.m`: ```objc // N/A ```

Android

Click To Expand

#### Have you converted to AndroidX? - [ ] my application is an AndroidX application? - [X] I am using `android/gradle.settings` `jetifier=true` for Android compatibility? - [X] I am using the NPM package `jetifier` for react-native compatibility? #### `android/build.gradle`: ```groovy // N/A ``` #### `android/app/build.gradle`: ```groovy // N/A ``` #### `android/settings.gradle`: ```groovy // N/A ``` #### `MainApplication.java`: ```java // N/A ``` #### `AndroidManifest.xml`: ```xml ```

Environment

Click To Expand

**`react-native info` output:** ``` System: OS: macOS 10.15.7 CPU: (8) x64 Intel(R) Core(TM) i5-1038NG7 CPU @ 2.00GHz Memory: 901.69 MB / 16.00 GB Shell: 5.7.1 - /bin/zsh Binaries: Node: 12.18.0 - /var/folders/hr/0hcq8l555yq2d6tlk0w_27xr0000gn/T/yarn--1604042363081-0.857331577094822/node Yarn: 1.22.4 - /var/folders/hr/0hcq8l555yq2d6tlk0w_27xr0000gn/T/yarn--1604042363081-0.857331577094822/yarn npm: 6.14.6 - ~/.nvm/versions/node/v12.18.0/bin/npm Watchman: Not Found Managers: CocoaPods: 1.9.3 - /usr/local/bin/pod SDKs: iOS SDK: Platforms: iOS 14.1, DriverKit 19.0, macOS 10.15, tvOS 14.0, watchOS 7.0 Android SDK: API Levels: 28, 29, 30 Build Tools: 28.0.3, 29.0.2, 30.0.0 System Images: android-27 | Google Play Intel x86 Atom, android-28 | Google APIs Intel x86 Atom_64, android-28 | Google Play Intel x86 Atom, android-29 | Google APIs Intel x86 Atom Android NDK: Not Found IDEs: Android Studio: 4.0 AI-193.6911.18.40.6626763 Xcode: 12.1/12A7403 - /usr/bin/xcodebuild Languages: Java: 1.8.0_272 - /usr/bin/javac Python: 2.7.16 - /usr/bin/python npmPackages: @react-native-community/cli: Not Found react: 16.13.1 => 16.13.1 react-native: 0.63.3 => 0.63.3 react-native-macos: Not Found npmGlobalPackages: *react-native*: Not Found ``` - **Platform that you're experiencing the issue on**: - [X] iOS - [X] 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:** - see `package.json` above - **`Firebase` module(s) you're using that has the issue:** - Analytics - **Are you using `TypeScript`?** - Y & 3.9.7

mikehardy commented 3 years ago

Hi there! Your versions are out of date, so you'll need to update them prior to any reproduction having actual predictive value. Please do that as a necessary first step then report back

It does seem like you're setting things correctly

You may also need to do a full / clean build before the firebase.json settings are ingested as I believe they are "rendered" at build time into the AndroidManifest and plist

https://firebase.google.com/docs/analytics/configure-data-collection?platform=ios (that is also how you could do this manually instead of relying on firebase.json to render it out, as a workaround)

Perhaps that is it. I'm not sure that is clear from the documentation - that a clean build is needed

At the same time I don't see the analytics settings rendered into the plist where I would expect them to be 🤔 https://github.com/invertase/react-native-firebase/blob/master/packages/app/ios_config.sh - though I might be missing exactly where things connect. Careful inspection of the plist post build to see what keys end up in there might be warranted

pehagg commented 3 years ago

Thanks for the reply @mikehardy, I'll look into this next week and will get back with the results.

stale[bot] commented 3 years ago

Hello 👋, to help manage issues we automatically close stale issues. This issue has been automatically marked as stale because it has not had activity for quite some time. Has this issue been fixed, or does it still require the community's attention?

This issue will be closed in 15 days if no further activity occurs. Thank you for your contributions.

pehagg commented 3 years ago

First of all, sorry for not coming back to this earlier, some higher priority issues came up.

Unfortunately, the behavior is still exactly the same after upgrading to 10.2.0. However, if I set FIREBASE_ANALYTICS_COLLECTION_ENABLED to false in Info.plist then it works as expected. I checked your codebase again and still would like to reiterate that analytics_auto_collection_enabled is not read anywhere. I see this only as a mere workaround, not as a proper fix for the issue.

Thanks for syncing the component versions, the previous setup with different component versions was a complete mess to try to keep up-to-date in relation to each other.

mikehardy commented 3 years ago

First of all, sorry for not coming back to this earlier, some higher priority issues came up.

No problem at all, I totally understand time constraints. Thanks for circling back and reporting

Unfortunately, the behavior is still exactly the same after upgrading to 10.2.0. However, if I set FIREBASE_ANALYTICS_COLLECTION_ENABLED to false in Info.plist then it works as expected. I checked your codebase again and still would like to reiterate that analytics_auto_collection_enabled is not read anywhere. I see this only as a mere workaround, not as a proper fix for the issue.

Okay, so the firebase-ios-sdk is working correctly (that's the plist toggle) but for some reason we haven't hooked up firebase.json to plist correctly as you mention. I'll re-title this and label it for further investigation

Note (related to time constraints) that the fastest way to resolve this will always be a PR, and there should be a pattern to follow with either crashlytics or admob ids going from firebase.json file to plist file

Thanks for syncing the component versions, the previous setup with different component versions was a complete mess to try to keep up-to-date in relation to each other.

Yeah - credit where it's due the firebase-ios-sdk team started it after asking users their pain points, and after they synchronized their versioning I decided it seemed like a positive step so did it here. I was worried people would be scared since it looks like a big version bump but it is quite nice as a maintainer too. Cheers

pehagg commented 3 years ago

I can confirm that doing the same for Android, i.e. adding the relevant key/value to AndroidManifest.xml had the same effect, i.e. opt-in started to work.

Would love to make a fix for this but there is just barely time to do all the worky-work stuff I need to do. Maybe, someday...