feat: Use a new RouterBuilder API for routes.tsx

[mshabarov]

Removes the buildRoutes method and replaces it with a new RouterBuilder API that automatically injects server side routes where needed and adds the auth level.

Related-to vaadin/hilla#2238

Fixes #18988

[github-actions]

Test Results

1 099 files  ±0  1 099 suites  ±0   1h 29m 41s ⏱️ + 4m 53s
6 946 tests +2  6 897 ✅ +2  49 💤 ±0  0 ❌ ±0 
7 280 runs  +7  7 219 ✅ +7  61 💤 ±0  0 ❌ ±0 

Results for commit b668bfd. ± Comparison against base commit 90abc04.

This pull request removes 9 and adds 11 tests. Note that renamed tests count towards both.

com.vaadin.flow.server.frontend.FrontendUtilsTest ‑ isHillaViewsUsed_buildRouteTsxWithoutArgsFSViewExists_false
com.vaadin.flow.server.frontend.FrontendUtilsTest ‑ isHillaViewsUsed_buildRouteTsxWithoutArgs_false
com.vaadin.flow.server.frontend.FrontendUtilsTest ‑ isHillaViewsUsed_buildRouteWithArgsTsx_true
com.vaadin.flow.server.frontend.TaskGenerateReactFilesTest ‑ routesContainImport_buildRoutes_noExceptionThrown
com.vaadin.flow.server.frontend.TaskGenerateReactFilesTest ‑ routesContainImport_serverSideRoutes_noExceptionThrown
com.vaadin.flow.server.frontend.TaskGenerateReactFilesTest ‑ routesContainsRoutesExport_noExceptionThrown
com.vaadin.flow.server.frontend.TaskGenerateReactFilesTest ‑ routesMissingImport_expectionThrown
com.vaadin.flow.server.frontend.TaskGenerateReactFilesTest ‑ routesMissingImport_noBuildOrServerSideRoutes_exceptionThrown
com.vaadin.flow.server.frontend.TaskGenerateReactFilesTest ‑ routesexportMissing_expectionThrown

com.vaadin.flow.server.frontend.FrontendUtilsTest ‑ isHillaViewsUsed_withFileRoutes_true
com.vaadin.flow.server.frontend.FrontendUtilsTest ‑ isHillaViewsUsed_withReactRoutes_true
com.vaadin.flow.server.frontend.TaskGenerateReactFilesTest ‑ routesContainImportAndUsage_serverSideRoutes_noExceptionThrown
com.vaadin.flow.server.frontend.TaskGenerateReactFilesTest ‑ routesContainNoImport_serverSideRoutes_exceptionThrown
com.vaadin.flow.server.frontend.TaskGenerateReactFilesTest ‑ routesContainOnlyImport_serverSideRoutes_exceptionThrown
com.vaadin.flow.server.frontend.TaskGenerateReactFilesTest ‑ routesExportMissing_exceptionThrown
com.vaadin.flow.server.frontend.TaskGenerateReactFilesTest ‑ routesMissingImportAndUsage_noBuildOrServerSideRoutes_exceptionThrown
com.vaadin.flow.server.frontend.TaskGenerateReactFilesTest ‑ routesMissingImport_exceptionThrown
com.vaadin.flow.server.frontend.TaskGenerateReactFilesTest ‑ withFallbackMissesImport_exceptionThrown
com.vaadin.flow.server.frontend.TaskGenerateReactFilesTest ‑ withFallbackMissing_exceptionThrown
…

♻️ This comment has been updated with latest results.

[tltv]

FrontendUtils#isRoutesTsxContentUsingHillaViews() has regex checks that should be changed now to support new format. Maybe it's enough to check if .withReactRoutes(...) is called.
Also part in FrontendUtils#isHillaViewsUsed(File) that check if there are any views in the Frontend/views/ should now check if .withFileRoutes(...) is actually called in addition to having views files.

[mshabarov]

FrontendUtils#isRoutesTsxContentUsingHillaViews() has regex checks that should be changed now to support new format. Maybe it's enough to check if .withReactRoutes(...) is called. Also part in FrontendUtils#isHillaViewsUsed(File) that check if there are any views in the Frontend/views/ should now check if .withFileRoutes(...) is actually called in addition to having views files.

Right, this is needed, will change.

[mshabarov]

FrontendUtils#isRoutesTsxContentUsingHillaViews() has regex checks that should be changed now to support new format

This now checks that routes.tsx has withFileRoutes or withReactRoutes method that means Hilla.

[mshabarov]

Tested with different configurations and routes.tsx content is as expected, also triggers an error if manual changes are incorrect.
Ready for review I think and should wait for RouterBuilder.

[sonarcloud]

Quality Gate passed

Issues
16 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[tltv]

I have this error in browser console with flow-hilla-hybrid-example, based on your branch use-routes-builder. Main page looked ok with one public view listed. Then sign in page gives me this when clicking Log in button:

[mshabarov]

I don't see this error, when logged in. This looks as something related to the LoginOverlay component.
Could you try in incognito mode or clear browser cache?

UPD: This might be related to AuthContext from hilla-react-auth, let me check.

[tltv]

UPD: This might be related to AuthContext from hilla-react-auth, let me check.

Not a cache issue as I get same error with Edge/Firefox. Yeah AuthContext is not setup for some reason. Maybe I run with old hilla-react-auth.

[mshabarov]

Try to clean up the project and re-run. I've just tested with the latest Hilla snapshot.

[mshabarov]

@caalador could you please also test?

[mshabarov]

@tltv could you please also double check if you have AuthProvider in the index.tsx?

import { createElement } from 'react';
import { createRoot } from 'react-dom/client';
import { RouterProvider } from 'react-router-dom';
import { AuthProvider} from "./auth";
import { router } from 'Frontend/routes.js';

function App() {
    return (
        <AuthProvider>
            <RouterProvider router={router} />
        </AuthProvider>
    );
}

createRoot(document.getElementById('outlet')!).render(createElement(App));

This is required.

[tltv]

@tltv could you please also double check if you have AuthProvider in the index.tsx?
...
This is required.

Ok I didn't use the custom index.tsx. It's not there in the generated index.tsx and somehow I expected it to be correct automatically. After copying default index.tsx and adjusting like that, it works. Shouldn't Flow auto-generate this part too?

[mshabarov]

This is problematic, as AuthProvider requires other codes that needs user information. You can check auth.tsx in the starter project. So we cannot easily add it to any project (into generated index file), because it is project-specific.

[mshabarov]

Why don't you use frontend/index.tsx file by the way? It should be in the vaadin/flow-hilla-hybrid-example#29

[mshabarov]

And even in the v24.4 branch https://github.com/vaadin/flow-hilla-hybrid-example/blob/v24.4/src/main/frontend/index.tsx.

[tltv]

Why don't you use frontend/index.tsx file by the way? It should be in the vaadin/flow-hilla-hybrid-example#29

I'm testing that generated files works too.

[tltv]

LGTM. Tested various use cases without facing any problems that require changes necessarily in this PR.
Listing some findings regarding the new API here:

• After removing .protect() from the routes.tsx, both root path and server routes with @AnonymousAllowed are available without requiring to log in. No matter what paths they are mapped to. This is also expected so.
• Without .protect(), all Hilla views are still protected and requires log in. Roles are just ignored. Probably due to the naming I expected all Hilla views to become public. Having protection still enabled is more secure, but is there some way to make client route public explicitly?
• Views added with withReactRoutes() are not listed in Available routes anymore (they were before builder api). Not registered to ClientRouteRegistry anymore?

[mshabarov]

These are good comments and something that we should investigate.
But not in this PR ofc, as this PR is about using the builder API and adapting to it.

[vaadin-bot]

This ticket/PR has been released with Vaadin 24.4.0.alpha21 and is also targeting the upcoming stable 24.4.0 version.