feat(file-router): add support for loaders

[cromoteca]

This PR is an attempt to allow File Router users to define a server method as data loader. Here's how to use it. Let's say we have a service with two methods (using @NonNullApi):

package org.vaadin.example.endpoints;

import com.vaadin.flow.server.auth.AnonymousAllowed;
import com.vaadin.hilla.Endpoint;

@Endpoint
@AnonymousAllowed
public class HelloEndpoint {
    public record Data(String name, String description, int quantity) {}

    public Data initialData() {
        return new Data("Awesome Product", "Amazing Description", 100);
    }

    public String repeat(String text, int times) {
        return text.repeat(times);
    }
}

The first method can be used as such:

import { useLoader } from "@vaadin/hilla-file-router/runtime.js";
import { ViewConfig } from "@vaadin/hilla-file-router/types.js";
import { HelloEndpoint } from "Frontend/generated/endpoints";

export const config = {
  title: "custom title",
  loader: HelloEndpoint.initialData,
} satisfies ViewConfig;

export default function MainView() {
  const { name, quantity } = useLoader(config);
  return <div className="p-m">{name} {quantity}</div>;
}

It is necessary to use satisfies ViewConfig instead of : ViewConfig so that useLoader can infer the right return type.

The second server method has parameters, so it's expected to be used in a route like /repeat/{text}/{times}. The name of the parameters is not important, it's the order that matters.

import { useLoader } from "@vaadin/hilla-file-router/runtime.js";
import { ViewConfig } from "@vaadin/hilla-file-router/types.js";
import { HelloEndpoint } from "Frontend/generated/endpoints";

export const config = {
    title: "Repeat",
    loader: HelloEndpoint.repeat,
} satisfies ViewConfig;

export default function RepeatView() {
    const repeated = useLoader(config);
    return <div className="p-m">{repeated}</div>;
}

In this case, repeated is a string. Calling /repeat/abc/3 will print abcabcabc in the view.

Server errors can potentially be handled at router level, allowing for example to return a 404 in case of user id not found.

Tests are missing: consider this PR as a base of discussion.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.91%. Comparing base (20b0aa0) to head (06bafdc).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #2562 +/- ## ========================================== - Coverage 95.00% 94.91% -0.10% ========================================== Files 66 66 Lines 4546 4562 +16 Branches 661 663 +2 ========================================== + Hits 4319 4330 +11 - Misses 182 187 +5 Partials 45 45 ``` | [Flag](https://app.codecov.io/gh/vaadin/hilla/pull/2562/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vaadin) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/vaadin/hilla/pull/2562/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vaadin) | `94.91% <100.00%> (-0.10%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vaadin#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[Legioth]

I'm a bit confused by the terminology here. I understand "loader" as a mechanism that ensures data needed by a view is loaded in parallel with the script needed for rendering the view. That cannot be the case here since the FS router doesn't even support lazy loading views. There's also a minor benefit from starting the request before running the view's render function but should be a very minimal impact in typical cases.

[cromoteca]

The terminology is borrowed from the React Router loader functionality that is used here. The benefits I can see are: avoid the need to use useEffect and the possibility to react to errors directly in the router, for example to return a 404 if /users/123 is not found.

[Legioth]

I'm a little hesitant about the satisfies ViewConfig syntax and I'm very much hesitant about the total lack of type safety when the service method takes parameters.

Would it be possible to come up with a syntax that avoids those issues? Doing that would probably somehow require a separate definition rather than building on top of export const config.

[cromoteca]

To avoid satisfies, a separate declaration is needed. Concerning type safety, I did an experiment with a @Loader annotation in Java, that generates different code in TypeScript allowing for parameter names to be associated by name instead of order. But, as parameters are always strings, either also server parameters are strings (which is not practical), or we accept that a conversion from strings will be attempted.

[Legioth]

Type conversion of parameters is indeed also a concern but my main concern was that it's only based on matching the order without any reference to parameter names.

[Legioth]

I wonder if there's some synergy with the concept that I started investigating related to adding type safety to FS route parameters? This would make the parameters more explicit in general and it would also have an option for defining parameter types. I wonder if something similar could also be used to map route parameters to loaders?

[cromoteca]

Sure! If we have type safety in parameters, that could be matched to server methods using some mechanism like the @Loader annotation I was talking about.

[Legioth]

I don't think a Java annotation would be very useful here since we're not generating Java code. We could just as well let every endpoint method be eligible to be used as a loader while hooking things up from TS code that can reference generated TS code.

If we assume that we generate a model based on the view's route parameters, then we could hook things up using something like loader(HelloEndpoint.repeat, model.text, model.times) assuming the type system can be abused in that way. Otherwise, there's the option of generating an entry point on all endpoint methods to enable use like HelloEndpoint.repeat.asLoader(model.text, model.times).

There might still be one step that isn't type-safe since the model (or more specifically its type) has to come from somewhere but that's something we might not be able to avoid.

[cromoteca]

Types could be specified in the config as generics. That would give something like:

export const config: ViewConfig<{ text: string, times: number }> = {
    title: "Repeat",
    loader: HelloEndpoint.repeat,
};

The idea of using an annotation was to change how code is generated for those annotated methods. So, a method like

  @Loader
  public User getUser(long id) throws UserNotFoundException {
    ...
  }

would be generated as

async function getUser({ params }: LoaderFunctionArgs): Promise<User> {
    return client.call('LoadersEndpoint', 'getUser', { id: Number(params.id) });
}

where LoaderFunctionArgs is the type used by React Router and UserNotFoundException extends our EndpointException.

[sonarcloud[bot]]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[Legioth]

I think it's problematic to generate @Loader methods in a different way since you might also want to use that method with parameters that are not derived from the URL. One obvious example is lazy loading of data - you wouldn't want to update the URL at the moment when the Grid component determines that more items need to be fetched but you would still want to use a loader for the initially fetched items. That's why I think it would be better to aim for a design where a regular endpoint method can also be used as a loader.

[Legioth]

In other words, what I'm leaning towards is something like TanStack Query (previously known as React Query) that is also used as a loader. Initial loading is triggered before the view is rendered based on router parameters and default values for any additional state. Subsequent updates to route parameters would update the URL whereas updates to other parameters would "only" trigger a re-fetch (and maybe also pushState?). Errors from the initial load could be handled as route errors (e.g. 401 or 404) whereas other errors (e.g. server timeout) would be handled by the view.

[Legioth]

If we want to also enable the waterfall busting benefit in combination with code splitting (i.e. lazy loaded view implementations), then the loader definition along with anything it depends on would have to be defined outside the actual view module. We can lift out immutable values from the view module at compile time but we might not want to do anything like that for e.g. arrow functions. This would put some additional constraints on the design even though we might also want to provide a simpler mode of operation that could have everything in a single file at the cost of a waterfall when combined with code splitting.