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.

Sign up for GitHub

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

Jump to bottom

fix: add replace for the new AD_SERVICES_CONFIG introduced in V18 #600

Merged
merged 3 commits into from
Jan 26, 2025
Merged

fix: add replace for the new AD_SERVICES_CONFIG introduced in V18 #600

merged 3 commits into from
Jan 26, 2025

Conversation

TheRogue76
Copy link
Contributor

@TheRogue76 TheRogue76 commented Jan 26, 2025 •
edited by mikehardy
Loading

Facebook SDK 18 introduced Google Privacy Sandbox adservices API, and when this SDK was updated to 18, that change was pulled in here in #595. This means that if a user has a package that tries to declare the same resource either A) The user has to do this override in their AndroidManifest or B) We do it here like how react-native-google-mobile-ads does it

Test Plan:

  1. I made the changes locally on my own project in Android Studio and made sure that it compiled successfully:
Screenshot 2025-01-26 at 09 01 40
  1. I made the change to the source code, changed the fork's example apps's npm package to github:TheRogue76/react-native-fbsdk-next and pulled my latest changes, made sure that it compiled successfully (Metro was giving warnings that the module doesn't exist in the simulator, but it did compile and i double checked that my manifest merger change in Android was present):
Screenshot 2025-01-26 at 09 03 49

Let me know if there is a better way of testing this locally. In Lottie, we link the package locally so changes could be pulled from the code right next to it so the example app can test what is already there

@TheRogue76 TheRogue76 changed the title chore: add override for the new AD_SERVICES_CONFIG introduced in V18 fix: add override for the new AD_SERVICES_CONFIG introduced in V18 Jan 26, 2025
@TheRogue76 TheRogue76 changed the title fix: add override for the new AD_SERVICES_CONFIG introduced in V18 fix: add replace for the new AD_SERVICES_CONFIG introduced in V18 Jan 26, 2025
@mikehardy mikehardy added the pending-merge Just waiting on clean CI test run. Any maintainer should feel free to merge if CI is passing. label Jan 26, 2025
mikehardy
mikehardy approved these changes Jan 26, 2025
View reviewed changes
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 great for the XML - I added in the explanatory comment but I think this will do the trick, thank you!

@mikehardy mikehardy merged commit d94ba92 into thebergamo:master Jan 26, 2025
5 checks passed
@mikehardy mikehardy removed the pending-merge Just waiting on clean CI test run. Any maintainer should feel free to merge if CI is passing. label Jan 26, 2025
github-actions bot pushed a commit that referenced this pull request Jan 26, 2025
@semantic-release-bot
chore(release): 13.4.1 [skip ci]
7bda1f8 
## [13.4.1](v13.4.0...v13.4.1) (2025-01-26)

### Bug Fixes

* **android:** handle AD_SERVICES_CONFIG manifest merge collision in SDK v18 ([#600](#600)) ([d94ba92](d94ba92))
@github-actions GitHub Actions
Copy link

🎉 This PR is included in version 13.4.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

@mikehardy mikehardy mikehardy approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Expo SDK 52 Android builds are failing: Manifest merger failed
2 participants
@TheRogue76 @mikehardy