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

[runtime] remove androidx.startup dependency #5761

Merged
merged 7 commits into from
Mar 25, 2024
Merged

Conversation

martinbonnin
Copy link
Contributor

@martinbonnin martinbonnin commented Mar 25, 2024

androidx.startup is not always available (unit tests for an example but also some other cases) and more generally, it's confusing to have the NetworkMonitor fail silently if androidx.startup doesn't work or if the READ_NETWORK_STATE permission is not granted.

In order to use failFastIfOffline and waitForNetwork(), you'll have to opt-in explicitely (I added some documentation here):

val apolloClient = ApolloClient.Builder()
  .serverUrl(...)
  // Android
  .networkMonitor(NetworkMonitor(context))
  // iOS
  .networkMonitor(NetworkMonitor())

It means you can't create a NetworkMonitor from commonMain, you'll have to expect/actual it but at this point, I think it's just more consistent.

Also take this opportunity to

See #5760
See #5720

  • remove androidx.startup dependency
  • initialize the ConnectivityManager in a background thread
  • focus the RetryInterceptor on Network errors, remove ApolloClient.Builder.retryOnErrorInterceptor
  • Add ensureUniqueUuids
  • apiDump

@martinbonnin martinbonnin requested a review from BoD as a code owner March 25, 2024 10:19
Copy link

netlify bot commented Mar 25, 2024

Deploy Preview for apollo-android-docs canceled.

Name Link
🔨 Latest commit e724e9c
🔍 Latest deploy log https://app.netlify.com/sites/apollo-android-docs/deploys/660150628235d30008688765

Copy link
Contributor

@BoD BoD left a comment

Choose a reason for hiding this comment

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

👍

@@ -21,6 +23,10 @@ fun Project.configureAndroid(
getCatalogVersion("android.sdkversion.min").toInt()
}
testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner"

if (this is LibraryVariantDimension) {
multiDexEnabled = true
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need 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.

The test APK grew over 64k methods 🤷 .

Screenshot 2024-03-25 at 13 30 53 Screenshot 2024-03-25 at 13 31 06

Interestingly, it's not the case on those 2 screenshots (I think? but somehow two dex files are still required 🤷 )
In all cases, it's close enough that it may happen again. Plus AGP enables it by default if minSdk > 21 anyways.

No place for degrowth in dex-land 🌱 🙈

@martinbonnin martinbonnin merged commit bdaae07 into main Mar 25, 2024
9 checks passed
@martinbonnin martinbonnin deleted the network-monitor-opt-in branch March 25, 2024 12:33
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.

None yet

2 participants