@NonNull changes in DataLoader APIs breaks downstream Kotlin apps.
Hi folks.
I wanted to bring to your attention the recent changes in version 4.0.0 where @NonNull has been added to the public load API.
Unlike Java, Kotlin enforces strict nullability checks at compile time, which has a significant impact on Kotlin users. This change requires Kotlin developers to refactor their code to handle nullability explicitly, which can be quite challenging across a large fleet of apps.
I noticed that the release notes do not specifically mention the rationale behind this change or if there’s an alternative way to achieve the desired functionality without altering the public APIs. If it's possible to avoid adding these @NonNull annotations to the public API, it would greatly ease the upgrade process for Kotlin users.
Thank you for your consideration and for all the hard work you put into maintaining this library.
Unlike Java, Kotlin enforces strict nullability checks at compile time, which has a significant impact on Kotlin users Can you please expand on how this affects you.
The graphql-java team is embracing JSpecify annotations to help produce better null check semantics and we can expect to see more of this happening dataloader and graphql-java itself.
I am keen to understand how you were impacted and what you needed to change.
4.0.0 was a breaking major version so breaking changes are nominally expected BUT we are keen to know how this affected you and what work you needed to do so we can try to avoid unnecessary pain as much as possible.
Hi,
With the addition of @NullMarked in this commit, also part of 5.0.0 (e.g., here), Kotlin now treats all unannotated generic type parameters as implicitly non-nullable. As a result, code that previously used nullable types for the value parameter V (e.g., DataLoader<K, V?>) now fails with a Type argument is not within its bounds: must be subtype of 'Any' error in Kotlin.
This is problematic for use cases where null is a valid data-fetcher result (e.g., missing values).
Would you consider either: Removing @NullMarked from the type level in favor of more granular annotations, or
Explicitly annotating V with @Nullable to allow nullability where appropriate?
This would restore compatibility with nullable Kotlin value types without compromising JSpecify intent.
Thanks
I think there are probably other places where nullness requires more refinements. For example, DataLoaderRegistry#getDataloader(String) should have a @Nullable return type.
I believe those can be considered as bugs and won't require a new major version. As a first step, configuring automated nullness checks should help find most issues. See an example setup here: https://github.com/bclozel/java-dataloader/commit/f449972110284ee350c7f1c7a8dac3daf28465ed
Thanks for the detailed explanations - this is indeed unintended. Perhaps we should put a Kotlin dependency into our tests just to make sure we compile better - both in dataloader and graphql-java
We will fix these up.
Kotlin now treats all unannotated generic type parameters as implicitly non-nullable. As a result, code that previously used nullable types for the value parameter V (e.g., DataLoader<K, V?>) now fails with a Type argument is not within its bounds: must be subtype of 'Any' error in Kotlin.
Explicitly annotating V with https://github.com/nullable to allow nullability where appropriate?
I would prefer this but you cant do
public class DataLoader<K, @Nullable V> {
because if its type use only.
@Documented
@Target(TYPE_USE)
@Retention(RUNTIME)
public @interface Nullable {}
This is interesting that Kotlin decided to make NullMarked here for the type parameters
I will look into this more.
I got an errorprone branch up and it does not do what Kotlin seems to be doing
Turns out you need to use this construct
@NullMarked
public class DataLoader<K, V extends @Nullable Object> {
The Kotlin compiler than decides that it can be
val dataLoader: DataLoader<String, String?> = DataLoaderFactory.newDataLoader { keys -> CompletableFuture.completedFuture(keys.toList()) }
I have added Kotlin example test files to test that we can compile and run as expected.
Adding to this thread just for visibility -- we tried bumping some deps but hit the same issue where our data loader, structured like:
class MyDataLoader<String, String?>
load(keys: Set<String>) {
// not all keys have a data entry
val data = getData(keys)
return keys.associateWith { data[it] }
}
was suddenly hit with the @NullMarked typing constraint
Hi, I see the issue was fixed for the DataLoader class itself, but I think it should be applied to more interfaces (we use these to define our custom data loaders) - https://github.com/graphql-java/java-dataloader/pull/206
Thanks everyone for the feedback. Could you try the newly released version 5.0.2?