Deprecate Coil and ImageLoader extension functions.

[colinrtwhite]

Looking for feedback on this!

This PR deprecates the ImageLoader.load(context, data), ImageLoader.get(data), Coil.load(context, data), and Coil.get(data) functions in favour of a more standard builder syntax:

// LoadRequest
val request = LoadRequest.Builder(context)
    .data("https://www.example.com/image.jpg")
    .target(imageView)
    .build()
val disposable = imageLoader.launch(request)

// GetRequest
val request = GetRequest.Builder()
    .data("https://www.example.com/image.jpg")
    .size(1080, 1920)
    .build()
val drawable = imageLoader.launch(request)

NOTE: This does not deprecate ImageView.load.

Why deprecate these functions? I covered a lot of my thoughts on builders here that I think apply here as well.

Upsides:

• Remove DSL syntax
• Java compatibility

Downsides:

• Type safety (data(Any))

Overall, I think this makes it clearer that requests are value objects that can be constructed now and executed later:

val request = LoadRequest.Builder(context)
    .data("https://www.example.com/image.jpg")
    .transformations(GrayscaleTransformation())
    .build()

// Then later...
val disposable = imageLoader.launch(request.newBuilder().target(imageView).build())

Open questions:

• Should there be a target + launch function similar to Glide and Picasso's into?
• Is it clear enough that you need to call data when constructing a request?

[Jawnnypoo]

Is it clear enough that you need to call data when constructing a request?

Is there a way to "block" the call until they call data, ie. they cannot continue with the chain until they do so? Do we throw exceptions if data is not called? That would probably be best, so that they know right away that they are not passing the data correctly.

[colinrtwhite]

@Jawnnypoo Yep, ideally we'd want to force data to be called as well. Unfortunately, I don't think it's possible to force it at compile time without adding an intermediary class that creates the Request.

Verifying it's called at runtime can be done one of two ways:

• Throw an exception as soon as build is called. This is how okhttp3.Request works as it requires you to call url before build. data(null) is technically correct usage so we'll have to check that the method was called instead of data being null. This also throws the exception at the callsite which is more explicit, but could be unexpected and/or cause crashes.
• Throw an exception when the request is launched. This is how it works already if Request.data is null. It also throws the exception into the coroutine scope so it won't crash, calls onError, and sets the fallback drawable. The drawback is this is more "silent".

I'm leaning towards the second since it's easier to go from 2 -> 1 (more restrictive) than it is to go from 1 -> 2 (less restrictive) if we decide we want to change. Let me know what you think!

[Jawnnypoo]

Ahhh yes, option 2 makes much more sense based on what is in place currently 👍

[gabrielittner]

Should there be a target + launch function similar to Glide and Picasso's into?

Did you make a decision on this?

[colinrtwhite]

@gabrielittner Yep, though definitely still looking for feedback if you agree/disagree. Originally in this PR request creation was going to be coupled with the ImageLoader - something like:

val disposable = imageLoader.load(context)
    .data("https://www.example.com/image.jpg")
    .target(imageView)
    .execute()

However, I ended up revising the PR based on feedback to keep the request separate from the ImageLoader:

val request = LoadRequest.Builder(context)
    .data("https://www.example.com/image.jpg")
    .target(imageView)
    .build()
val disposable = imageLoader.execute(request)

Since the RequestBuilder doesn't have a reference to an ImageLoader any more it doesn't make sense to have a target + execute function.