Skip to content

Conversation

radko93
Copy link
Contributor

@radko93 radko93 commented Jul 9, 2025

Description

fixes #8605

This is according to https://firebase.google.com/docs/analytics/configure-data-collection?platform=ios#disable-IDFA-collection

Related issues

Release Summary

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
    • Other (macOS, web)
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Test Plan


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

Copy link

vercel bot commented Jul 9, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-firebase ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 9, 2025 6:23pm

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.

I think we always need FirebaseAnalytics/Core as mentioned in suggestion comment

Comment on lines 42 to +44
if defined?($RNFirebaseAnalyticsWithoutAdIdSupport) && ($RNFirebaseAnalyticsWithoutAdIdSupport == true)
Pod::UI.puts "#{s.name}: Not installing FirebaseAnalytics/IdentitySupport Pod, no IDFA will be collected."
s.dependency 'FirebaseAnalytics/Core', firebase_sdk_version
Copy link
Collaborator

Choose a reason for hiding this comment

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

In the new pod arrangement for FirebaseAnalytics pods + sub-pods, combined with our desire to allow folks to have fine-grained control with Podfile variable toggles so that all parts are optional, we need to start with FirebaseAnalytics/Core for all arrangements and then go from there

I think it may work to have the Core sub-spec in the conditional because of transitive dependencies bringing it in some of the conditional branches where it is needed, but...

I think it's most correct to always start with it as a dependency, outside of all conditionals, and then go

What do you think @radko93 ? I think this is most correct and if I don't hear back I'm going to commit and release it so we don't have the broken 22.3.0 version out too long...

Suggested change
if defined?($RNFirebaseAnalyticsWithoutAdIdSupport) && ($RNFirebaseAnalyticsWithoutAdIdSupport == true)
Pod::UI.puts "#{s.name}: Not installing FirebaseAnalytics/IdentitySupport Pod, no IDFA will be collected."
s.dependency 'FirebaseAnalytics/Core', firebase_sdk_version
s.dependency 'FirebaseAnalytics/Core', firebase_sdk_version
if defined?($RNFirebaseAnalyticsWithoutAdIdSupport) && ($RNFirebaseAnalyticsWithoutAdIdSupport == true)
Pod::UI.puts "#{s.name}: Not installing FirebaseAnalytics/IdentitySupport Pod, no IDFA will be collected."

Copy link
Collaborator

Choose a reason for hiding this comment

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

going with the other PR, but I appreciate this and sorry for the trouble!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants