feat(file-router): add RouterBuilder class #2250

Lodin · 2024-03-21T18:26:31Z

codecov · 2024-03-21T18:32:45Z

Codecov Report

Attention: Patch coverage is 97.43590% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 95.35%. Comparing base (4624ded) to head (a359549).

Files	Patch %	Lines
...e-router/src/runtime/RouterConfigurationBuilder.ts	97.08%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2250      +/-   ##
==========================================
+ Coverage   93.79%   95.35%   +1.55%     
==========================================
  Files          66       67       +1     
  Lines        1644     4431    +2787     
  Branches      370      630     +260     
==========================================
+ Hits         1542     4225    +2683     
- Misses         67      163      +96     
- Partials       35       43       +8

Flag	Coverage Δ
unittests	`95.35% <97.43%> (+1.55%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mshabarov · 2024-03-22T14:30:41Z

Flow counterpart is here vaadin/flow#19020.
Let's check if the naming/signatures are correspond in these two PRs.

mshabarov · 2024-03-25T06:28:47Z

Is the API in this PR going to be changed according to #2238 ?

platosha · 2024-03-26T12:04:12Z

Is the API in this PR going to be changed according to #2238 ?

Yes.

# Conflicts: # .mocharc.cjs # package-lock.json # packages/ts/file-router/package.json # packages/ts/file-router/src/runtime/RouterBuilder.ts # packages/ts/file-router/src/shared/traverse.ts # packages/ts/file-router/test/runtime/RouterBuilder.spec.tsx

mshabarov · 2024-03-27T15:10:55Z

Tested this branch with hybrid starter got the following problems:

views.js is still generated instead of expected file-routes.js
don't know how I should import RoutesBuilder, import {RouterBuilder} from "@vaadin/hilla-file-router/runtime.js"; doesn't work, nor doesn't work the autogenerated imprort below.
I get compilation errors in Flow.tsx

2024-03-27T17:03:47.944+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :  ERROR(TypeScript)  Argument of type 'unknown[]' is not assignable to parameter of type 'AgnosticRouteObject[]'.
2024-03-27T17:03:47.944+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :  FILE  /Users/mikhail/workspace/vaadin/flow-hilla-hybrid-example/src/main/frontend/generated/flow/Flow.tsx:278:39
2024-03-27T17:03:47.944+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   : 
2024-03-27T17:03:47.944+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :     276 |             }
2024-03-27T17:03:47.945+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :     277 |
2024-03-27T17:03:47.945+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :   > 278 |             let matched = matchRoutes(Array.from(routes), window.location.pathname);
2024-03-27T17:03:47.945+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :         |                                       ^^^^^^^^^^^^^^^^^^
2024-03-27T17:03:47.945+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :     279 |
2024-03-27T17:03:47.945+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :     280 |             // if router force navigated using 'Link' we will need to remove
2024-03-27T17:03:47.947+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :     281 |             // flow from the view
2024-03-27T17:03:47.947+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   : 
2024-03-27T17:03:47.948+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :  ERROR(TypeScript)  Cannot find module 'Frontend/generated/file-routes' or its corresponding type declarations.
2024-03-27T17:03:47.948+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :  FILE  /Users/mikhail/workspace/vaadin/flow-hilla-hybrid-example/src/main/frontend/routes.tsx:32:24
2024-03-27T17:03:47.950+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   : 
2024-03-27T17:03:47.951+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :     30 |  ******************************************************************************/
2024-03-27T17:03:47.951+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :     31 | import { serverSideRoutes } from 'Frontend/generated/flow/Flow';
2024-03-27T17:03:47.953+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :   > 32 | import fileRoutes from 'Frontend/generated/file-routes';
2024-03-27T17:03:47.954+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :        |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2024-03-27T17:03:47.954+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :     33 | import Login from "Frontend/views/login";
2024-03-27T17:03:47.954+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :     34 | import RouterBuilder from "@vaadin/hilla-file-router/runtime/RouterBuilder";
2024-03-27T17:03:47.956+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :     35 |
2024-03-27T17:03:47.956+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   : 
2024-03-27T17:03:47.956+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :  ERROR(TypeScript)  Cannot find module '@vaadin/hilla-file-router/runtime/RouterBuilder' or its corresponding type declarations.
2024-03-27T17:03:47.956+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :  FILE  /Users/mikhail/workspace/vaadin/flow-hilla-hybrid-example/src/main/frontend/routes.tsx:34:27
2024-03-27T17:03:47.958+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   : 
2024-03-27T17:03:47.959+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :     32 | import fileRoutes from 'Frontend/generated/file-routes';
2024-03-27T17:03:47.959+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :     33 | import Login from "Frontend/views/login";
2024-03-27T17:03:47.959+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :   > 34 | import RouterBuilder from "@vaadin/hilla-file-router/runtime/RouterBuilder";
2024-03-27T17:03:47.962+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :        |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2024-03-27T17:03:47.962+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :     35 |
2024-03-27T17:03:47.962+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :     36 | const routerBuilder = new RouterBuilder()
2024-03-27T17:03:47.962+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :     37 |     .withFileRoutes(fileRoutes)
2024-03-27T17:03:48.007+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   : 
2024-03-27T17:03:48.007+02:00  INFO 51748 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   : [TypeScript] Found 3 errors. Watching for file changes.

mshabarov · 2024-03-27T15:13:57Z

Also, the routes in views.js have still the "" mapping. But they shouldn't as it conflicts with AtRoute("") in Flow.

Lodin · 2024-03-27T15:19:15Z

views.js is still generated instead of expected file-routes.js

It should be in another PR, I guess.

don't know how I should import RoutesBuilder

Oops, my bad. Fixed

I get compilation errors in Flow.tsx

That's could probably be solved by fixing two previous points.

Also, the routes in views.js have still the "" mapping. But they shouldn't as it conflicts with AtRoute("") in Flow.

Well... How it is supposed to be used?

packages/ts/file-router/src/runtime/RouterBuilder.ts

…state

mshabarov · 2024-03-28T07:18:05Z

Tested again with the Flow-Hilla starter and with the latest changes in this PR:

Used Flow branch that uses RouterBuilder API;
Used the following codes in routes.tsx:

import  Flow  from 'Frontend/generated/flow/Flow';
import { serverSideRoutes } from 'Frontend/generated/flow/Flow';
import fileRoutes from 'Frontend/generated/views';
import Login from "Frontend/views/login";
import {RouterBuilder} from "@vaadin/hilla-file-router/runtime.js";


const routerBuilder = new RouterBuilder()
    .withFileRoutes(fileRoutes)
    .withReactRoutes(
        { path: '/login', element: <Login />, handle: { title: 'Login' } },
    )
    .withServerRoutes(Flow)
    .protect('/login');

export const routes = routerBuilder.routes;

export default routerBuilder.build();

~~Got the following issues:~~

ERROR(TypeScript)  Argument of type 'Element' is not assignable to parameter of type 'ComponentType'.
2024-03-28T08:50:19.844+02:00  INFO 59108 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :  FILE  /Users/mikhail/workspace/vaadin/flow-hilla-hybrid-example/src/main/frontend/routes.tsx:43:23
2024-03-28T08:50:19.844+02:00  INFO 59108 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   : 
2024-03-28T08:50:19.845+02:00  INFO 59108 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :     41 |         { path: '/login', element: <Login />, handle: { title: 'Login' } },
2024-03-28T08:50:19.845+02:00  INFO 59108 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :     42 |     )
2024-03-28T08:50:19.845+02:00  INFO 59108 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :   > 43 |     .withServerRoutes(Flow())
2024-03-28T08:50:19.846+02:00  INFO 59108 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :        |                       ^^^^^^
2024-03-28T08:50:19.846+02:00  INFO 59108 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :     44 |     .protect('/login');
2024-03-28T08:50:19.846+02:00  INFO 59108 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :     45 |
2024-03-28T08:50:19.848+02:00  INFO 59108 --- [v-server-output] c.v.b.devserver.DevServerOutputTracker   :     46 | export const routes = routerBuilder.routes;

~~This is probably easy to fix by declaring ComponentType type for Flow, but I don't know how at the moment.~~
UPD: the issue above isn't reproduced anymore.

If I ~~ignore this TS error,~~ rebuild and navigate to localhost:8080/flow or localhost:8080/hilla, it redirects me to the login page. Then, when I logged in, I can navigate to /hilla and /flow, these are rendered fine, but without the main layout 🤔

Also, navigating to / gives me a blank page.

mshabarov · 2024-03-28T11:20:17Z

Noticed that if I change routes config to

    .withReactRoutes(
        { path: '', element: <Public />, handle: { title: 'Login' } },
        { path: '/login', element: <Login />, handle: { title: 'Login' } }
    )

then wherever I navigate (/, /hilla or /flow), I get navigated to Public view.

mshabarov · 2024-03-28T11:21:38Z

And this issue with ViewConfig #2276 is observable for this PR as well.

packages/ts/file-router/src/runtime/RouterBuilder.ts

mshabarov · 2024-04-04T12:44:14Z

Have you tested it with the hybrid example?

I get the same issues (blank root page localhost:8080/ and no main layout) with:

Building a plugin from this branch with

npm install
npm run build -w @vaadin/hilla-file-router
npm pack -w @vaadin/hilla-file-router

Build Flow from feat: Use a new RouterBuilder API for routes.tsx flow#19020
Checkout to a branch feat: Use RoutesBuilder API flow-hilla-hybrid-example#29, run, stop
Install local plugin with npm i {path-to-local-packed-plugin-tgz}
Run app with mvn

mshabarov · 2024-04-04T12:44:52Z

Here is my routes config:

import Flow from 'Frontend/generated/flow/Flow';
import fileRoutes from 'Frontend/generated/views';
import Login from "Frontend/views/login";
import {RouterConfigurationBuilder} from "@vaadin/hilla-file-router/runtime.js";

export const { routes, router} = new RouterConfigurationBuilder()
    .withFileRoutes(fileRoutes)
    .withReactRoutes(
        { path: '/login', element: <Login />, handle: { title: 'Login' } }
    )
    .withFallback(Flow)
    .protect('/login')
    .build();

mshabarov · 2024-04-04T14:04:52Z

My bad.

The latest version uses @ instead of $ in the naming convention.
After fixing it, the main layout and public page is shown as expected.

But now the Flow view is shown with the Public view content.
This is probably happening, because FS router injects "*" child element with Flow component into Public route, becuase it thinks that it's a layout (maybe my explanation isn't precise enough, but the screenshot below should explain better):

mshabarov · 2024-04-04T14:10:00Z

This is how it looks like:

Screen.Recording.2024-04-04.at.17.08.20.mov

packages/ts/file-router/src/runtime.ts

# Conflicts: # packages/ts/file-router/package.json

…e/builder

…ing adding fallback

mshabarov · 2024-04-08T13:49:52Z

With the latest changes the app works as expected.
Well done @Lodin @platosha 🥇

packages/ts/file-router/test/vite-plugin/createViewConfigJson.spec.ts

sonarcloud · 2024-04-09T07:14:46Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

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

See analysis details on SonarCloud

resolved

feat(file-router): add RouterBuilder class

043b539

Lodin added the hilla Issues related to Hilla label Mar 21, 2024

mshabarov mentioned this pull request Mar 22, 2024

feat: Use a new RouterBuilder API for routes.tsx vaadin/flow#19020

Merged

Lodin added 3 commits March 26, 2024 17:08

refactor(file-router): implement Builder class

47fefa8

refactor(file-router): finalize RouterBuilder & add tests

c11b122

Lodin marked this pull request as ready for review March 27, 2024 14:01

fix: export RouterBuilder correctly

571f1dc

cromoteca previously requested changes Mar 27, 2024

View reviewed changes

packages/ts/file-router/src/runtime/RouterBuilder.ts Outdated Show resolved Hide resolved

packages/ts/file-router/src/runtime/RouterBuilder.ts Outdated Show resolved Hide resolved

Lodin added 2 commits March 27, 2024 17:35

fix: use React component instead of element to avoid possible shared …

e399dde

…state

refactor(file-router): remove unnecessary method

77a8244

taefi mentioned this pull request Mar 28, 2024

refactor: rename views.ts to file-routes.ts #2271

Merged

mshabarov reviewed Mar 28, 2024

View reviewed changes

packages/ts/file-router/src/runtime/RouterBuilder.ts Outdated Show resolved Hide resolved

mshabarov mentioned this pull request Mar 28, 2024

ViewConfig has no effect for authentication properties #2276

Closed

Lodin added 2 commits April 4, 2024 12:31

refactor(file-router): tune RouterBuilder API

3335b1e

Merge branch 'main' into feat/file/builder

ae0467c

platosha requested changes Apr 5, 2024

View reviewed changes

packages/ts/file-router/src/runtime.ts Show resolved Hide resolved

platosha requested changes Apr 8, 2024

View reviewed changes

packages/ts/file-router/src/runtime.ts Show resolved Hide resolved

platosha and others added 7 commits April 8, 2024 09:49

fix: restore toReactRouter export

2e0c623

Merge branch 'main' into feat/file/builder

180d6c4

Merge branch 'main' into feat/file/builder

1e9312c

Merge branch 'main' into feat/file/builder

d82a55e

Merge branch 'refs/heads/main' into feat/file/builder

0b21146

# Conflicts: # packages/ts/file-router/package.json

Merge remote-tracking branch 'origin/feat/file/builder' into feat/fil…

49e1a42

…e/builder

refactor(file-router): fix adding children routes to a leaf route dur…

961ab56

…ing adding fallback

platosha requested a review from mshabarov April 8, 2024 13:13

mshabarov previously approved these changes Apr 8, 2024

View reviewed changes

Lodin requested a review from platosha April 8, 2024 16:30

platosha previously approved these changes Apr 9, 2024

View reviewed changes

Merge branch 'main' into feat/file/builder

36b36ab

platosha dismissed stale reviews from mshabarov and themself via 36b36ab April 9, 2024 06:41

platosha requested changes Apr 9, 2024

View reviewed changes

packages/ts/file-router/test/vite-plugin/createViewConfigJson.spec.ts Outdated Show resolved Hide resolved

platosha added 2 commits April 9, 2024 09:48

chore(file-router): fix syntax in createViewConfigJson.spec.ts

2b3e257

test(file-router): correct test fixtures

a359549

platosha approved these changes Apr 9, 2024

View reviewed changes

platosha merged commit 55e161e into main Apr 9, 2024
15 checks passed

platosha deleted the feat/file/builder branch April 9, 2024 07:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(file-router): add RouterBuilder class #2250

feat(file-router): add RouterBuilder class #2250

Lodin commented Mar 21, 2024 •

edited by platosha

codecov bot commented Mar 21, 2024 •

edited

mshabarov commented Mar 22, 2024

mshabarov commented Mar 25, 2024

platosha commented Mar 26, 2024

mshabarov commented Mar 27, 2024

mshabarov commented Mar 27, 2024

Lodin commented Mar 27, 2024

mshabarov commented Mar 28, 2024 •

edited

mshabarov commented Mar 28, 2024

mshabarov commented Mar 28, 2024

mshabarov commented Apr 4, 2024

mshabarov commented Apr 4, 2024

mshabarov commented Apr 4, 2024

mshabarov commented Apr 4, 2024

mshabarov commented Apr 8, 2024

sonarcloud bot commented Apr 9, 2024

feat(file-router): add RouterBuilder class #2250

feat(file-router): add RouterBuilder class #2250

Conversation

Lodin commented Mar 21, 2024 • edited by platosha

codecov bot commented Mar 21, 2024 • edited

Codecov Report

mshabarov commented Mar 22, 2024

mshabarov commented Mar 25, 2024

platosha commented Mar 26, 2024

mshabarov commented Mar 27, 2024

mshabarov commented Mar 27, 2024

Lodin commented Mar 27, 2024

mshabarov commented Mar 28, 2024 • edited

mshabarov commented Mar 28, 2024

mshabarov commented Mar 28, 2024

mshabarov commented Apr 4, 2024

mshabarov commented Apr 4, 2024

mshabarov commented Apr 4, 2024

mshabarov commented Apr 4, 2024

mshabarov commented Apr 8, 2024

sonarcloud bot commented Apr 9, 2024

Quality Gate passed

Lodin commented Mar 21, 2024 •

edited by platosha

codecov bot commented Mar 21, 2024 •

edited

mshabarov commented Mar 28, 2024 •

edited