-
Notifications
You must be signed in to change notification settings - Fork 41.1k
Add jackson-module-kotlin to spring-boot-starter-json #9803
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
I think that's a good idea, and that would provide a much simpler solution to this issue discussed in spring-io/initializr#462 + would also works for people not using start.spring.io, with small drawback (adding 40 Kb JAR) which is not much more impacting than shipping Kotlin bytecode in Spring JARs like we already do on Framework and Data sides. It does not prevent non-Kotlin projects to work since Spring Framework already checks if Kotlin is in the classpath before registering the module. |
Seems sensible to me. |
Thanks @bclozel, that will help a huge number of Kotlin developers !!! |
I'd like to question the decision as it now leads to every Java based web project to ship a Kotlin integration dependency. Projects that don't care about Kotlin at all. Projects that make up the vast majority of our user base. Consider the fact that you're upgrading from 1.x and now end up with additional artifacts in your classpath that you never ever gonna need. To me, the impact to unrelated parties is way too high considering the amount of convenience gained for a very small minority of our user base. |
To me that's not very different than Spring Framework or Spring Data shipping Kotlin bytecode and support into their JAR. Jackson has done a different choice regarding to Kotlin support packaging, that's why we ship it by default with Boot 2 otherwise Spring + Kotlin projects are broken. Notice the module is present but not registered when Kotlin is not on the classpath. The alternative to current arrangement would have been to add such dependency only for Kotlin project on start.spring.io and has been rejected with some good arguments I think. Current arrangement has the advantage to be consistent with our strategy: builtin support for Kotlin without tricky dependency management which is IMO one of the purpose of Boot regardless start.spring.io help. |
It's difficult to discuss this without "way too high" being quantified. We deliberately exclude the module's Kotlin dependencies, so the effect for a Java user is that there's an extra 47KB jar file on their application's class path that they won't use. As @sdeleuze highlights above, in terms of runtime functionality, the presence of that jar file will have no effect whatsoever. In terms of runtime performance, the presence of that jar file may have an effect as it's one extra jar that needs to be searched when loading a class. I've created a couple of minimal web applications using Boot 2.0.0.M5 to compare things. The jars sizes are as expected: 15498528 bytes with the Kotlin module and 15450117 bytes without. That's an increase of 48411 bytes (47.28KB) which is 0.31%. Startup performance seems to be unaffected. Using Spring Boot 2.0.0.M5 and the web starter, the average startup time was 2.592 seconds. The same app, but with the Jackson Kotlin module excluded started, on average, in 2.583 seconds. That's a whole 0.009 seconds quicker with the Kotlin module. It's comfortably within the error bars so the startup performance can be considered to be identical. We already considered the 47KB impact above and agreed that we were comfortable with it. Beyond that, I'm struggling to find any impact at all. Perhaps I've missed something, but, in the absence of evidence of any other impacts, I can't see any reason to change our decision here. |
The thing is: you open up the question "why not do it for other things as well?" and where do you draw the line then? 40k here, 40k there. In contrast to the Kotlin Spring integration, the JAR being a separate one allows plain-Java projects to not have the code on the classpath. That's a feature, not a bug. If there's no argument to actually have it on the classpath except: "it doesn't matter", then that's a pretty weak one I think. There's a much higher chance that people will decide to use a JSR-303 they might have not used before than there is for the usage of that Kotlin-specific JAR in a Java project, as that chance is 0 by definition. It's a plain matter of dependency hygiene that I wouldn't want JARs sneaking into my binary that I know I will never ever need. There's one other aspect, that makes me think that all those Kotlin specialties shouldn't make it into Boot this way: you won't get a Kotlin based project up and running without touching the POM significantly. Beyond that there are quite a few additional dependencies that you need to declare to reasonably work with it as well. Does Boot add all of those to Java projects as well? If not, why not? Maybe because the non-test JARs of that add a whopping 3,3 MB to the classpath. 4 more lines don't make this much worse. Especially if they – just as the build setup – can be added by the Initializer, that actually has knowledge about the language used to be precise about what to include. It might be a bit more work on our side but the easiest solution for us isn't necessarily the best in general. My point is: the current story is inconsistent in a way that we include some additional libs that make stuff work, but not all of them. Or are you going to include those aforementioned additional libs in the Spring Data JPA starter because they're needed so that this works? |
I am wondering: what if we had I know they'd create additional affort but would also allow us to create a more complete story, rather than leaving build setup to users, some dependency setup but not other setup. |
@olivergierke Your new argument applies equally to decisions that you've made in Spring Data. You have included support for Vavr and Scala in Spring Data Commons rather than shipping separate modules. I believe you also decided to do the same for Spring Data's Kotlin support. Separate modules would have provided the feature that non- Scala, Kotlin, and Vavr users avoid having code that they don't need on the classpath. Why didn't you do so? |
Because its in line with what Spring Framework has decided. My key points are:
|
We are using the starter to make it appear that Jackson's Kotlin-specific classes are part of a core Jackson module, thereby, for users of the starter, making things consistent with what's been done in Framework and Data. No one has claimed that this issue provides a complete Kotlin dependencies story and there may well be more work to do. However, we believe that what we've done here is a step in the right direction.
I agree, although I think the support should be split across Boot and Spring Initialzr. I believe the latter already does a good job of setting up your pom.xml or build.gradle ready to build a Kotlin project. If you are aware of other areas where the Kotlin experience could be improved, please open separate tickets for those so that we can build on what's been done in this issue. |
I'm starting using 2.0-M6 and was a bit confused, that after configuring Jackson ObjectMapper via Jackson2ObjectMapperBuilder and with enabled findModulesViaServiceLoader-Feature I got runtime errors because of missing kotlin classes. The workaround was to exclude jackson-kotlin dependency from web-starter. Will required kotlin-dependencies for jackson-kotlin-module enabled by default in the final 2.0 release? I would argue that there are only two possible solutions: mark all kotlin-related libs as optional or all required dependencies must be already enabled. |
@gbrehmer thanks for the feedback. Can you please create a new issue. A sample that demonstrates the problem would be idea. |
done: #11133 |
Spring Framework, Spring Data, Reactor and manny other projects ship Kotlin extensions in their JARs. Other projects, like Jackson, choose to provide Kotlin support as separate artifacts.
Right now (see initializr#462), creating a Spring Boot application with Kotlin can be misleading, since you're getting full support from the Spring projects but not quite for JSON serialization.
Should we add that 40Kb JAR in the
spring-boot-starter-json
?We'd need to exclude the transitive dependency
org.jetbrains.kotlin:kotlin-reflect
, as this should be provided by the project that choses to use the Kotlin language. Spring Framework should defensively register that module only if the Kotlin library is present.Note that other libraries might choose to do the same (ship Kotlin support as separate artifacts), so this decision might affect other libraries we already support.
The text was updated successfully, but these errors were encountered: