hilla icon indicating copy to clipboard operation
hilla copied to clipboard

Generator outputs `undefined` as a possible return type for endpoint methods calling `client.subscribe`

Open cromoteca opened this issue 3 years ago • 2 comments

In Hilla 1.1, when generating TypeScript endpoints with Flux return types (i.e. push), the return type includes undefined. For example, this code:

public Flux<User> getAllUsers() {
    return Flux.just();
}

is translated as:

function getAllUsers(): Subscription<User | undefined> | undefined {
  return client.subscribe('FluxTestEndpoint', 'getAllUsers', {});
}

Now, as @Lodin pointed out, client.subscribe returns Subscription<any>, so the last | undefined shouldn't be there.

The multi-module generator does not make the same mistake.

cromoteca avatar Aug 05 '22 13:08 cromoteca

I expect | undefined in TypeScript, cause there was no @Nonnull mentioned. Is this a bug in the multi-module generator then?

platosha avatar Aug 08 '22 16:08 platosha

OK I see, Subscription is an asynchronous wrapper type, it is meaningless to return undefined in the client here. @Nonnull rules should not apply then.

platosha avatar Aug 09 '22 12:08 platosha

So what happens now if you return ”null”?

Artur- avatar Aug 17 '22 12:08 Artur-

Actually, nothing changes, as client.subscribe always returns something, and this change is just to reflect that in TypeScript endpoint return types.

Then, if someone returns null in theirs Flux-related endpoint methods, he'll get an error on the Java side, with or without this fix. I don't remember the details now, but the Java implementation does not handle nulls and will throw NPE.

As for the client part, see https://github.com/vaadin/hilla/blob/e2f4dc35147235e0ab6ec93ffb25dfbea334d6ee/packages/ts/hilla-frontend/src/Connect.ts#L444-L446

and https://github.com/vaadin/hilla/blob/e2f4dc35147235e0ab6ec93ffb25dfbea334d6ee/packages/ts/hilla-frontend/src/FluxConnection.ts#L124

cromoteca avatar Aug 17 '22 13:08 cromoteca