hilla icon indicating copy to clipboard operation
hilla copied to clipboard

[24.8] Kotlin Nullability plugin does not take default argument values into account

Open grimsi opened this issue 6 months ago • 4 comments

Describe the bug

While using the 24.8.0.alpha6 pre-release version I noticed that the new Kotlin nullability plugin doesn't take default arguments into account. In my opinion they should be marked as optional in the frontend since TypeScript also supports optional parameters. Detecting if a parameter is optional should be possible with KParameter.isOptional() from kotlin-reflect. I admit this is probably a minor issue that could also be fixed by refactoring the Kotlin endpoint code, but I still wanted to report it.

Additional sources:

  • https://kotlinlang.org/docs/functions.html#default-arguments
  • https://www.typescriptlang.org/docs/handbook/2/functions.html#optional-parameters
  • https://kotlinlang.org/api/core/kotlin-stdlib/kotlin.reflect/-k-parameter/is-optional.html

Expected-behavior

This Kotlin code:

@Endpoint
@PermitAll
class TestEndpoint() {

    fun sayHello(name: String = "World"): String {
        return "Hello $name!"
    }
}

Should result in this TypeScript code:

async function sayHello_1(name: string | undefined, init?: EndpointRequestInit_1): Promise<string> { return client_1.call("GameEndpoint", "sayHello", { name }, init); }

or even this:

async function sayHello_1(name?: string, init?: EndpointRequestInit_1): Promise<string> { return client_1.call("GameEndpoint", "sayHello", { name }, init); }

Reproduction

The actual generated code (notice the type of the parameter "name"):

async function sayHello_1(name: string, init?: EndpointRequestInit_1): Promise<string> { return client_1.call("GameEndpoint", "sayHello", { name }, init); }

System Info

Vaadin platform version: 24.8.0.alpha6 Kotlin version: 2.1.20

grimsi avatar May 09 '25 13:05 grimsi

@grimsi Thanks for creating this issue. To my understanding, proper handling of optional parameters wasn't in the scope of supporting Kotlin's native nullability from the beginning, but I agree that supporting this would be a nice addition to the Hilla's generator, since the concept of optional params exists in both worlds.

Considering nullability vs. optionality, I'm personally in favor of

async function sayHello_1(name?: string, init?: EndpointRequestInit_1): Promise<string> { 
  return client_1.call("GameEndpoint", "sayHello", { name }, init); 
}

as it enables the caller to perform sayHello() instead of sayHello(undefined).

taefi avatar May 12 '25 07:05 taefi

It would require some effort from our side to support this use case, and it's probably not coming in soon. There are missing related features in the parser, such as proper support for method overloads, for instance.

platosha avatar May 13 '25 11:05 platosha

Let me elaborate a bit on that. While we are improving Kotlin support, we are still calling Java methods in response to client requests. That sayHello function is mapped to a Java method with a non-optional parameter. You should add the @JvmOverloads annotation to create an overload without the name parameter, but then we have the problem that method overloading is not supported in Hilla for reasons like some(int n) and some(double n) would both map to some(n: number) and similar caveats. This does not mean that supporting this feature is impossible, but not that easy either.

cromoteca avatar May 13 '25 12:05 cromoteca

Thanks for the responses! I See that this is more complicated than I initially thought. It's a pretty minor issue anyway and easily fixable by refactoring the Kotlin code in the endpoint.

grimsi avatar May 13 '25 17:05 grimsi