rjaros / kvision

Object oriented web framework for Kotlin/JS
https://kvision.io
MIT License
1.2k stars 66 forks source link

kvision gradle plugin not works well with Spring-Web-Flux #527

Closed LiYing2010 closed 1 month ago

LiYing2010 commented 1 month ago

looks like there is a func getServerType() in source code of kvision gradle plugin

https://github.com/rjaros/kvision/blob/e9f3eb843777fbb4176c9986e3f4aa425654cbaf/kvision-tools/kvision-gradle-plugin/src/main/kotlin/io/kvision/gradle/KVisionPlugin.kt#L605-L607

but it only checks the dependency name spring-boot-starter-web, which is for Spring Web MVC

my project is using spring-boot-starter-webflux, which is for Spring Web Flux, and it does not works well with kvision gradle plugin

I guess it's because of func getServerType() does not support spring-boot-starter-webflux

I suggest to add spring-boot-starter-webflux into func getServerType(), how do you think?

if you agree, I will make PR to fix this

rjaros commented 1 month ago

Hello. This check is just a simple addon, contributed by KVision user not long ago: https://github.com/rjaros/kvision/pull/516 In general KVision plugin detects Spring Boot by kvision-server-spring-boot module dependency. I understand you don't use this module and still want to use KVision tasks for Spring Boot. If so, we can add starter-webflux detection. Feel free to make a PR.

LiYing2010 commented 1 month ago

If so, we can add starter-webflux detection

I made PR #528 to fix this please review when you get time thanks

In general KVision plugin detects Spring Boot by kvision-server-spring-boot module dependency. I understand you don't use this module and still want to use KVision tasks for Spring Boot.

Yes, I read this doc 6. Full Stack Development Guide / Common Code / Build configuration

and noticed that I should add kvision-server-spring-boot as an api dependency to source set commonMain

by in my own opinion, (1) commonMain is a common source set, which is used by both JS and JVM platform (2) but foo-server-bar dependency should be used only by JVM, since its name imply it is for Server Side

that's why I feel a little strange about JS module depends on a foo-server-bar dependency

actually, the dependency report for jsCompileClasspath and jsRuntimeClasspath are:

+--- io.kvision:kvision-server-spring-boot:7.4.5
|    \--- io.kvision:kvision-server-spring-boot-js:7.4.5
|         +--- io.kvision:kvision-common-annotations:7.4.5
|         |    \--- io.kvision:kvision-common-annotations-js:7.4.5
|         +--- io.kvision:kvision-common-types:7.4.5
|         |    \--- io.kvision:kvision-common-types-js:7.4.5
|         +--- io.kvision:kvision-common-remote:7.4.5
|         |    \--- io.kvision:kvision-common-remote-js:7.4.5

they depend on only kvision-common-foo-js, none of foo-server-bar dependency is used

rjaros commented 1 month ago

The kvision-server-spring-boot dependency is a multiplatform module. It contains code for all three targets - common, JS and JVM. Thats why it needs to be added to the common module. But you are right, this name is probably a bit confusing. I've chosen a different name in my new Kilua RPC project, which extracts KVision fullstack code into a separate project.

rjaros commented 1 month ago

Fixed in 7.5.x