coil-kt / coil

Image loading for Android and Compose Multiplatform.
https://coil-kt.github.io/coil/
Apache License 2.0
10.73k stars 651 forks source link

HttpUrl isn't supported in Coil 3.x. #2540

Open colinrtwhite opened 5 days ago

colinrtwhite commented 5 days ago

Describe the bug In Coil 2.x I could load an OkHttp HttpUrl like so:

AsyncImage(
    model = "https://example.com/image.jpg".toHttpUrl(),
    contentDescription = null,
)

however, that doesn't work on Coil 3.x. I've already imported io.coil-kt.coil3:coil-network-okhttp:3.0.0-rc01.

Version Coil 3.0.0-rc01

colinrtwhite commented 5 days ago

Opening this for visibility on an issue found in 3.0.0-rc01. Currently we use a service loader to add in the OkHttpNetworkFetcherFactory when io.coil-kt.coil3:coil-network-okhttp is imported. This is fine for Fetchers and Decoders as we can delay looking them up in the service loader (which is slow without R8) until we're on a background thread. In Coil 2.x, we could reference HttpUrl directly since Coil was coupled with OkHttp so this wasn't an issue.

To add default support for HttpUrl in Coil 3.x we'd have to service load an HttpUrlMapper, however by default Mappers run on the main thread so we'd need to check the service loader on the main thread. This is pretty slow and something I'd want to avoid by default. I'm leaning to not supporting this in 3.x since it's easy to add support for this on your own if you need it; just register this Mapper:

import coil3.map.Mapper
import coil3.request.Options
import okhttp3.HttpUrl

class HttpUrlMapper : Mapper<HttpUrl, String> {
    override fun map(data: HttpUrl, options: Options) = data.toString()
}

You can also map it at the callsite:

AsyncImage(
    model = myHttpUrl.toString(),
    contentDescription = null,
)

Thoughts, feedback, concerns? Post it in this thread!