hilla icon indicating copy to clipboard operation
hilla copied to clipboard

Include server-side routes path into views.js automatically

Open mshabarov opened this issue 1 year ago • 3 comments

Describe your motivation

Would be more convenient for users if the serverSideRoutes are added into views.js (FS routes generated routes metadata) by default taking into account the routes hierarchy.

Basically this is needed to avoid codes like this

views.children?.push(...serverSideRoutes);

in the routes.tsx.

If the routes hierarchy doesn't have children, the server side routes aren't added. I'd prefer this configuration happens automatically in views.js file or at least in other genereted file hidden from me.

Describe the solution you'd like

Add serverSideRoutes to routes config in views.js

Describe alternatives you've considered

No response

Additional context

Hilla 24.4.

mshabarov avatar Feb 29 '24 07:02 mshabarov

Should be possible to do this automatically based on the routing metadata that is included in the index HTML document. We should just take care to make the decision based on whether there are any server-side routes accessible for the user even if those are routes that wouldn't be shown in the menu (i.e. because they have required parameters).

Legioth avatar Feb 29 '24 07:02 Legioth

I'm wondering if we need to address this.

The issue we face with automation here is that it's essentially impossible to anticipate the layout a user might prefer. Given that MainLayout is merely a $layout in the root of the frontend/views directory, we can't predict where users will place their serverSideRoutes. They might put it in the root $layout, a nested $layout, or they might even want to place serverSideRoutes entirely outside of the layout.

If we were to forcibly insert serverSideRoutes into the root $layout, it could cause numerous issues for users trying to remove them. Furthermore, the root $layout might not even exist.

Lodin avatar Mar 05 '24 14:03 Lodin

The default should be to render server-side views in the top-level $layout but it should also be possible for the application developer to inject ...serverSideRoutes into some other part of the router configuration. We have not previously supported having different server-side views in different client-side layouts and we will not add this feature now either.

Since we're moving towards having routes.tsx in frontend/generated/ by default, we can force the serverSideRoutes reference into that file whenever there exist server-side routes. If the developer chooses to copy that file into frontend/ and customize it, then they won't benefit from the automatic addition and removal of serverSideRoutes but would instead have to do that manually if they want to start or stop using Flow views.

Legioth avatar Mar 06 '24 07:03 Legioth

We discussed it further offline and that's how the nearest plan look like:

  • [ ] Run FS router Vite plugin always to have at least empty routes array from views.js that routes.tsx can use.
  • [ ] Generate the same default codes content of routes.tsx no matter whether it's Flow, Hilla or both in frontend/generated, if this file doesn't exist in frontend/.
  • [ ] toReactRouter function includes children property to the output route object always, so the default codes can use route.children?.push(...serverSideRoutes); always.
  • [ ] Provide an instruction in the default content how can developer fallback to explicit routes config and place server-side routes into arbitrary level in the hierarchy
export const routes = [
  {
    element: <MainLayout />,
    handle: { title: 'Main' },
    children: [
      { path: '/hilla', element: <HillaView />, handle: { title: 'Hilla' } },
      ...serverSideRoutes
    ],
  },
] as RouteObject[];
  • [ ] If developer wants to modify routes config, he moves the routes.tsx file from frontend/generated to frontend/ and Flow in this cases shouldn't override it's content and dynamically update import of this file in Flow.tsx.
  • [ ] FS router docs gives details and instruction how to reconfigure the routes.

mshabarov avatar Mar 07 '24 08:03 mshabarov

and dynamically update import of this file in Flow.tsx.

I guess this is related to the circular dependency that we discussed?

We cannot make any assumption about what will be exported from frontend/routes.tsx when the application developer has modified it based on their own needs. This also means that we cannot import it from Flow.tsx even if there wouldn't be a circular dependency. We need to deal with that in a different way although that can be the topic of another ticket since things wouldn't change for the worse in this regard if implemented according to what's described here.

Legioth avatar Mar 07 '24 13:03 Legioth

After discussion in the Hilla team, here are the things we should hopefully achieve in the end:

  • default generated routes.tsx works with both file routes and server routes and applies protectRoutes
  • the content should be clear, so that the user can easily customize it
  • rename views.js to file-routes.js
  • rename the Flow import to server-routes.js
  • replace createBrowserRouter with our own API (createVaadinRouter(), for example), so that we could customize its behavior in future

platosha avatar Mar 19 '24 12:03 platosha

I've extracted the currently missing Hilla parts as #2238 and #2239. Then let us address the routes.tsx content in the scope of this issue.

platosha avatar Mar 20 '24 11:03 platosha

The remaining routes.tsx part was addressed by vaadin/flow#19020, closing.

platosha avatar Apr 10 '24 08:04 platosha