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

apple rejection on kids app: Messaging and DynamicLinks have a dependency of Firebase/Analytics #4072

Closed
pabloearg opened this issue Aug 11, 2020 · 15 comments · Fixed by #4131
Labels
help: ios Needs help implementing or reviewing a PR relating to iOS code. platform: ios plugin: analytics Google Analytics for Firebase Workflow: Waiting for User Response Blocked waiting for user response.

Comments

@pabloearg
Copy link

pabloearg commented Aug 11, 2020

Issue

we just update to the latest version of:

    "@react-native-firebase/dynamic-links": "7.5.0",
    "@react-native-firebase/messaging": "7.5.0",

but Apple has a new rule for kids: https://developer.apple.com/app-store/review/guidelines/#kids-category
in which you can't have any dependency to analytics, so, is really necessary to have this dependencies ?

  # Firebase dependencies
 _s.dependency          'Firebase/Analytics', firebase_sdk_version_ (remove this line)
  s.dependency          'Firebase/Messaging', firebase_sdk_version

  # Firebase dependencies
  s.dependency          'Firebase/Analytics', firebase_sdk_version (remove this line)
  s.dependency          'Firebase/DynamicLinks', firebase_sdk_version

can i remove this lines from the podspecs?

or do you have any solution to this?

maybe this issues are related:
firebase/firebase-ios-sdk#1686
firebase/firebase-ios-sdk#5153
firebase/firebase-ios-sdk#5928

@mikehardy
Copy link
Collaborator

@pabloearg honestly, I'm not sure? I recommend using https://github.com/ds300/patch-package in conjunction with altering the .podspec files to remove those lines and seeing how it works

You linked what appear to me (as I read them) to be all the relvant issues, especially 5153, which indicates that it's an area of recently active development, so it might be that we are just not "caught up" here and in your testing it works (with patch-package keeping it working) and we'd take a PR here

@pabloearg
Copy link
Author

@mikehardy reading this issues specifically this comment is says they release a new version with this, and following their releases, they fix this in the 6.27.1 but in the latest version of the plugin you have this:

  • RNFBDynamicLinks (7.5.0):
    • Firebase/Analytics (~> 6.27.0)
    • Firebase/DynamicLinks (~> 6.27.0)
    • React
    • RNFBApp
  • RNFBMessaging (7.5.0):
    • Firebase/Analytics (~> 6.27.0)
    • Firebase/Messaging (~> 6.27.0)
    • React
    • RNFBApp

@andersonaddo
Copy link
Contributor

I think you're outdated?
We're using ios sdk 6.28.1
#3956

@mikehardy
Copy link
Collaborator

@pabloearg did you attempt the build without analytics in the podspecs? I'm curious how it went

@mikehardy mikehardy added Workflow: Waiting for User Response Blocked waiting for user response. help: ios Needs help implementing or reviewing a PR relating to iOS code. platform: ios plugin: analytics Google Analytics for Firebase labels Aug 12, 2020
@pabloearg
Copy link
Author

pabloearg commented Aug 12, 2020

@pabloearg did you attempt the build without analytics in the podspecs? I'm curious how it went

@mikehardy yes, but only with messages (i removed the @react-native-firebase/dynamic-links package), the archive went good, and we received the push notifications, but apple continues to rejecting our app (without telling us which package is doing that)

@mikehardy
Copy link
Collaborator

I'm really sorry to hear that, but thank you for reporting in. This is obviously an area of current concern, I'll be listening for any future info but I thought that would be it so I'm stumped at the moment

@Salakar
Copy link
Member

Salakar commented Aug 12, 2020

Check the Podfile.lock file, it has a dependency graph, should show whats pulling in analytics

@pabloearg
Copy link
Author

I'm really sorry to hear that, but thank you for reporting in. This is obviously an area of current concern, I'll be listening for any future info but I thought that would be it so I'm stumped at the moment

@mikehardy after removing analytics from podspec and removing another package (branch.io) apple approve our new version.

for you to take it into account: we also removed the analytics dependency from @react-native-firebase/dynamic-links, and worked perfectly. (now we are sending a new version with dynamic-links to check if its work also)

could you also remove that dependency? so we dont need to fork or hardcode you podspecs

@mikehardy
Copy link
Collaborator

Excellent news! We can do anything that we need to do in order to make sure it's possible to approve things, I'm just waiting on reports of "what functionality is working" vs "what changes were made" vs "apple approved or not", which you are providing (thank you!), so I'll wait a bit longer to now to hear how dynamic-links works but https://github.com/ds300/patch-package is the way to do it so you don't have to fork or worry about hard-coding (it's very surgical!) and we'll get the needed changes in once we know what works

@Salakar
Copy link
Member

Salakar commented Aug 13, 2020

The main reason its included in links podspec is because that's what the iOS Firebase docs suggest: https://firebase.google.com/docs/dynamic-links/ios/create#set-up-firebase-and-the-dynamic-links-sdk

It doesn't indicate though if its actually required, it may just be that its there for analytics related to links/sharing links etc

@mikehardy
Copy link
Collaborator

Appears to be useful but not required https://firebase.google.com/docs/dynamic-links/analytics

I'm guessing this is just a lag between the (frankly, quite large) amount of Pod / package reorganization done to have Analytics be optional and what current requirements and recommendations are documented.

Sounds like RNFBDynamicLinks.podspec could have it removed, and rnfirebase.io page for Dynamic Links could get a pointer to the page I link here with a note that Analytics is optional but works together with Dynamic Links to provide more information if desired.

I believe the exact same applies to Messaging (remove from podspec, indicate Analytics is optional but can work together with Messaging to provide more messaging-related information - https://firebase.google.com/docs/analytics#integrations_with_other_services )

@Salakar
Copy link
Member

Salakar commented Aug 13, 2020

I think a PR to do that is in order then 🤔

@pabloearg are you able to send a PR for this?

@mikehardy
Copy link
Collaborator

For anyone reading this later: firebase/crashlytics@8.3.4 or greater will be required if you want to use crashlytics and still put your app in the kids category, I think it's the last change required.

@mikehardy
Copy link
Collaborator

This may be of interest to people. firebase-ios-sdk 7.8.0 has apparently made enough changes that Analytics is possible from within an iOS-acceptable kids app? firebase/firebase-ios-sdk#7652

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help: ios Needs help implementing or reviewing a PR relating to iOS code. platform: ios plugin: analytics Google Analytics for Firebase Workflow: Waiting for User Response Blocked waiting for user response.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants