Skip to content

[Fix] Make androidx.metrics:metrics-performance:1.0.0-beta01 resolution more robust (RN <0.76) #937

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

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft

[Fix] Make androidx.metrics:metrics-performance:1.0.0-beta01 resolution more robust (RN <0.76) #937

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion packages/core/android/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ dependencies {
// From 2.21.0, it uses 1.0.0-beta02, which requires Gradle 8.6.0.
// This breaks builds if the React Native target is below 0.76.0. as it relies on Gradle 8.5.0.
// To avoid this, we enforce 1.0.0-beta01 on RN < 0.76.0
if (reactNativeMinorVersion < 76) {
if (reactNativeMajorVersion == 0 && reactNativeMinorVersion < 76) {
implementation("com.datadoghq:dd-sdk-android-rum:2.23.0") {
exclude group: "androidx.metrics", module: "metrics-performance"
}
Comment on lines +199 to 202
Copy link
Member

Choose a reason for hiding this comment

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

why this is still needed if resolutionStrategy is used anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one makes sure that the right version gets used while compiling the SDK. Without it (and just with the resolution strategy that I just added) the build would fail because that resolution would arrive too late. I tested it in many ways and this was the only way to get it to work with the RN build pipeline.

The new addition is just there to make sure that if any other dependency included on the host app also introduces this dependency we resolve it to beta01, as the default resolution strategy is to choose the higher version, and we don't want that.

It's a bit convoluted, I know 😕

Copy link
Member

Choose a reason for hiding this comment

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

This is strange, something is not right. You should have only implementation("com.datadoghq:dd-sdk-android-rum:2.23.0"), without exclude.

Try to put resolutionStrategy call before dependencies are added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also tried that, yes. It didn't work either. That way it would always end up using beta02 and ignoring the RN version.

That's why I had to use this approach of excluding android.metrics from com.datadoghq:dd-sdk-android-rum and then adding beta01 on my own. Obviously this is only for React Native versions under 0.76, for anything newer we simply do implementation "com.datadoghq:dd-sdk-android-rum:2.23.0" on the line below.

Copy link
Member

Choose a reason for hiding this comment

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

It should work, so it is very strange, probably something is not right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll give it another shot, see If I can figure out a way to make it work 🦾

Copy link
Member

Choose a reason for hiding this comment

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

@sbarrio Any updates on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not yet, I'll let you know as soon as I have some.

In the meantime, I'm moving this back to draft to avoid confusion 😅

Expand Down Expand Up @@ -286,4 +286,17 @@ project.afterEvaluate {
project.android.unitTestVariants.all { variant ->
variant.registerPreJavacGeneratedBytecode(outputJarDependency)
}

// dd-sdk-android-rum requires androidx.metrics:metrics-performance.
// From 2.21.0, it uses 1.0.0-beta02, which requires Gradle 8.6.0.
// This breaks builds if the React Native target is below 0.76.0. as it relies on Gradle 8.5.0.
// To avoid this, we enforce 1.0.0-beta01 on RN < 0.76.0
// This resolution ensures that version 1.0.0-beta01 will be used even if another dependency also includes androidx.metrics:metrics-performance
if (reactNativeMajorVersion == 0 && reactNativeMinorVersion < 76) {
configurations.all {
resolutionStrategy {
force 'androidx.metrics:metrics-performance:1.0.0-beta01'
}
}
}
}
Loading