Deprecate DSL syntax in favour of Builder syntax.

[colinrtwhite]

Looking for feedback on this!

TLDR:

val loader = ImageLoader(context) {
    availableMemoryPercentage(0.5)
    crossfade(true)
    componentRegistry {
        if (SDK_INT >= P) {
            add(ImageDecoderDecoder())
        } else {
            add(GifDecoder())
        }
        add(SvgDecoder(context))
    }
}

is becoming this:

val loader = ImageLoader.Builder(context)
    .availableMemoryPercentage(0.5)
    .crossfade(true)
    .componentRegistry {
        if (SDK_INT >= P) {
            add(ImageDecoderDecoder())
        } else {
            add(GifDecoder())
        }
        add(SvgDecoder(context))
    }
    .build()

When I started Coil I wanted to experiment with DSLs since I thought they could replace builders in Kotlin. Since then I've come to the opinion that DSLs are great, but they don't always replace builders:

• DSLs are best if you need to describe document structure/ordering of elements (e.g. HTML DSL, Jetpack Compose). Coil's DSL isn't ordered.

• DSLs mix the receiver's scope with the current scope. This means IDE autocomplete is slower as the set of available functions is much larger. It's also less clear what functions are part of the builder and what functions are part of the outer scope. Chaining functions on a builder is a much smaller scope with less functions.

• DSLs are designed to create a single instance. Builders are factories. Eventually, I'd like to get to a point where it's possible to share MemoryCaches and BitmapPools between ImageLoader instances similar to OkHttp's newBuilder. A DSL syntax doesn't support this well.

• Method return type doesn't matter with DLSs. With builders you can use a method's return type of restrict/augment the functions available further down the chain. With DSLs every method calls the same receiver type.

• Builders are compatible with Java.

• Consumers can still create extension functions on top of the builder if they prefer the DSL.

Also Google, Square, Facebook, et al. continue to use builders in Kotlin - likely for some of these reasons.

TODO: Update the docs.

[Jawnnypoo]

I think this makes sense, people are more used to the builder pattern, as like you mentioned it is used more widespread. It also allows for more API exploration, as you can type builder. and IDE will make suggestions of all the possible methods to call.

Is there a reason to deprecate vs. just supporting both and letting people decide which they like better syntactically?

[colinrtwhite]

Is there a reason to deprecate vs. just supporting both and letting people decide which they like better syntactically?

Mostly to avoid confusion. By not including the DSL syntax it's also a subtle nudge to use the builder syntax. I'm also trying to track pretty closely with AndroidX, Square, and other libraries and Coil would be the only one shipping with both a builder + DSL syntax.

Still unsure of the best way to deprecate the invoke functions given people who want to keep the DSL should be able to define an extension function and have it resolve over the deprecated function. What do you think?

[Jawnnypoo]

Which invoke functions are you referring to, and what is the resolution issue?

[colinrtwhite]

@Jawnnypoo If someone wants to continue to use the DSL, but doesn't want it to show as deprecated they should define their own ImageLoader.Companion.invoke extension function, however currently it'll always be shadowed by the deprecated ImageLoader.Companion.invoke function that's part of the class. Unfortunately, I'm not sure there's a way to fix that without removing the deprecated ImageLoader.Companion.invoke function or settings its deprecation level to DeprecationLevel.HIDDEN.

[Jawnnypoo]

I see. Well, you could probably just remove without deprecating, since the library is not yet at a 1.0.0. Not ideal of course, but if it is going to limit what others can do, it might be best.

[colinrtwhite]

@Jawnnypoo Sounds good. I've marked it as DeprecationLevel.HIDDEN so at least consumers will be able to define their own extension functions. It's also still part of the code so users can figure out what changed without reading the release notes.

[dafi]

I love DSL when nesting elements helps to understand the relation parent <-> child, inside Coil this is less evident and I'm not a huge fan of Coil-DSL, I prefer the old-way builders.

+1 For removing DSL directly without passing from deprecation