Remove getHighlightsAsync and the kotlinx-coroutines dependency.

[yoxjames]

I want to start off saying that this is a really cool and interesting project and I think a KMP based syntax highlighter is an awesome idea! I am excited to use it for something I am working on. However, I need to recommend a reversion of a recently added change.

getHighlightsAsync(...) should be removed from this project along with the dependency on kotlinx-coroutines. This dependency is a massive penalty for bundle sizes and for targets like Javascript this could make or break usability of a project. I realize this might be somewhat difficult now at version 1.0.0 (I am proposing a binary incompatible change).

If I am writing Kotlin and am fine with the dependency I am free to still use coroutines of course:

    runBlocking {
        val highlights = Highlights.Builder()
            .code(sampleClass)
            .theme(SyntaxThemes.monokai())
            .language(SyntaxLanguage.JAVA)
            .build()

        val asncResult = async {
            val highlights = Highlights.default().run {
                setCode("public class ExampleClass {}")
                getHighlights()
            }
        }

        //..... (some time later)

       asyncResult.await()
    }

I could use a try/catch (or Supervisor Job or CoroutineExceptionHandler) for errors getting what the callback provided. The only piece that would be missing is that I cannot utilize cooperative cancellation between getCodeStructure() and constructHighlights(...) since constructHighlights(...) is private. If that was not the case, I would be able to use something like ensureActive() like you did in my callsite example. Realistically I am unsure how big of a problem that is (it wouldn't be for my use case but others may disagree). Alternatively getHighlights(...) could have an overload (or default param) to take a CodeStructure object allowing callers to implement the exact same cooperative cancellation from the outside.

I could also wrap this in something like a Future on JVM or Promise on a JS target without bringing in any new dependencies if I was concerned with bundle size.

I don't think this library should take an opinion on async processing and simply let the caller deal with that however they wish. Javascript users might use Promises, Java users Futures, etc. I get that this is simply a convenience function that can be ignored, but it's really the dependency I want to highlight. As a library developer, I care a lot about transitive dependencies and I don't think it's safe to assume that everyone who uses this also has kotlinx-coroutines in their dependency graph or wants to add it.

[tmaxxdd]

@yoxjames Thank you for so detailed message!

I have been working quite long on async version with coroutines as this was on the top of request, especially for implementation in KodeView and I think many people can benefit from this.

I have also started thinking about connection with JS lately and this gives me a point to think about it.

You gave completely valid arguments but I cannot say for now that I make immediate revert of coroutines.

I mentioned in other threads that this library is a base for code views libraries in other technologies. I will decide what to do taking everything into consideration.

I am really grateful that you have found this project so interesting to write this issue :)

[yoxjames]

As for async processing, I think that can still be done inside KodeView. Jetpack Compose already depends on coroutines and has functionality like LaunchedEffect based on suspend so it fits naturally into that space. I think the same argument can also be said for kotlinx-serialization but I didn't want to bog down my original post too much. I am not sure that all consumers would actually need serialization or would necessarily want to use kotlinx-serialization. My use case involves using this to generate static HTML with CSS styling. Therefore serialization would not be needed. There's no client/server component.

However it is 100% your project and I do think that since it's at 1.0 and this is a binary incompatible change, it's not exactly an easy call. I wanted to bring it up as I had a pretty different use case in mind (static HTML generation). If there is any interest in this I would be happy to do a PR even for just a proof of concept that you are free to ignore until a 2.0 release is ready :).

[Nek-12]

I feel like the coroutines dependency can just be a separate module. Solves both problems?