Conversation
Coil 3 auto-registers OkHttpNetworkFetcherFactory via ServiceLoader, which R8 can strip in release builds, leaving HTTP image loading without a fetcher and rendering only placeholders. Adding it directly to the component registry removes the ServiceLoader dependency for the default SDK path. (cherry picked from commit 183e0f6)
(cherry picked from commit 22aa113)
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
WalkthroughThe changes explicitly register Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/images/StreamImageLoaderFactory.kt (1)
38-48:⚠️ Potential issue | 🟡 MinorMinor: KDoc claim about "cache control and optimized network dispatcher" no longer matches the code.
The class KDoc still advertises "OkHttp with cache control and optimized network dispatcher", but
OkHttpNetworkFetcherFactory()is now constructed with noOkHttpClient/cache configuration, so Coil will use its defaults. Consider either passing a configuredOkHttpClient(with the cache/dispatcher you intend) or trimming the KDoc to reflect what's actually configured here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/images/StreamImageLoaderFactory.kt` around lines 38 - 48, The KDoc for StreamImageLoaderFactory is out of date: it claims "OkHttp with cache control and optimized network dispatcher" but the implementation constructs OkHttpNetworkFetcherFactory() without a configured OkHttpClient; either supply a preconfigured OkHttpClient (with cache and custom Dispatcher) into OkHttpNetworkFetcherFactory when building the ImageLoader, or remove/adjust that phrase in the KDoc to accurately reflect that Coil's default OkHttp client is used; locate the StreamImageLoaderFactory class and the OkHttpNetworkFetcherFactory() usage to apply the fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/images/StreamImageLoaderFactory.kt`:
- Around line 38-48: The KDoc for StreamImageLoaderFactory is out of date: it
claims "OkHttp with cache control and optimized network dispatcher" but the
implementation constructs OkHttpNetworkFetcherFactory() without a configured
OkHttpClient; either supply a preconfigured OkHttpClient (with cache and custom
Dispatcher) into OkHttpNetworkFetcherFactory when building the ImageLoader, or
remove/adjust that phrase in the KDoc to accurately reflect that Coil's default
OkHttp client is used; locate the StreamImageLoaderFactory class and the
OkHttpNetworkFetcherFactory() usage to apply the fix.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 934302c1-4a95-4462-a426-88abba85239b
📒 Files selected for processing (2)
stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/images/StreamImageLoaderFactory.ktstream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/images/StreamImageLoaderFactoryTest.kt
SDK Size Comparison 📏
|
andremion
changed the title
|
2 participants
Goal
Since the Coil 2 → Coil 3 migration in 6.13.0, customers have reported blank/placeholder images in release builds (support ticket). Coil 3 registers
OkHttpNetworkFetcherFactoryviaServiceLoader, which R8 full-mode can strip, leaving theImageLoaderwithout a fetcher for HTTP URLs. Registering it explicitly removes theServiceLoaderdependency.Implementation
StreamImageLoaderFactory— addOkHttpNetworkFetcherFactory()to the components block. Integrators delegating toStreamImageLoaderFactory(ComposeStreamCoilImageLoaderFactory.defaultFactory()and the XMLStreamCoilsingleton) pick up the fix for free. No public API changes.Not included — consumer R8 keep rules for Coil 3
ServiceLoaderdescriptors. With the fetcher registered in code, those only matter for integrators building a fully customImageLoader, which is already covered by the docs.Testing
./gradlew :stream-chat-android-ui-common:testDebugUnitTest --tests "StreamImageLoaderFactoryTest"— includes a new assertion thatfetcherFactoriescontains aNetworkFetcher.Factory.stream-chat-android-compose-sampleon a device that previously reproduced the blank-image symptom, open a channel with image attachments, confirm images load without any integrator-side fetcher registration.No UI changes.
Summary by CodeRabbit