hilla icon indicating copy to clipboard operation
hilla copied to clipboard

White Screen / Blank View in routes if view is not imported

Open sugihartolim opened this issue 2 years ago • 5 comments

Description of the bug

Using the hilla-grocery-app example from the tutorial, When creating or adding a new route to "routes.ts", it would compile just fine, but showing blank/white/empty view with no error messages / warnings on both backend and frontend.

export const views: ViewRoute[] = [
    // place routes below (more info https://vaadin.com/docs/latest/fusion/routing/overview)
    {
        path: '',
        component: 'grocery-view',
        icon: '',
        title: '',
    },
    {
        path: 'grocery',
        component: 'grocery-view',
        icon: 'la la-list-alt',
        title: 'Grocery',
    },
    {
        path: 'contact',   // <------------- NEW ROUTE 
        component: 'contact-view',
        icon: 'la la-list-alt',
        title: 'Contact',
    },
]; 

For the new route to be recognized, the view needs to be imported. If the line below does not exist, you'll get the blank view.

import { Route } from '@vaadin/router';
import './views/grocery/grocery-view';
import './views/main-layout';
import './views/contact-view';   // <------------ NEED THIS LINE 

Minimal reproducible example

As Above.

Expected behavior

Some checks should be performed and an error message should be displayed if the route is unknown or the view cannot be found.

Better yet, maybe don't use strings for the component reference and trigger an error during compilation. (Cf: maybe compare how the Angular router uses actual class/view object reference, not a string.)

Actual behavior

No errors/warnings, blank view.

Versions:

- Hilla 1.0.0 / 1.0.1 / 1.0. 2 

sugihartolim avatar Mar 30 '22 14:03 sugihartolim

Perhaps relatedly, no error is generated if the view is imported, but the component name in the routes is misspelled.

tarekoraby avatar Mar 30 '22 14:03 tarekoraby

A minimal fix for that — the router could check if the component is registered under the name while rendering navigation results.

Optimally, we should probably support using component class names in the route map, so that TypeScript does static checking here.

platosha avatar Apr 05 '22 11:04 platosha

How would class name references work together with lazy importing the view?

Artur- avatar Apr 05 '22 12:04 Artur-

Thanks for pointing this out. From the top of my head, we could support setting component constructor name references lazily then, while TypeScript ensures that the lazy function promises a component constructor. Here is a TypeScript playground prototype:

type ComponentCtor = {new(): HTMLElement};
type LazyComponentCtor = () => Promise<ComponentCtor>;

type Route = {
    path: string,
    componentClass: ComponentCtor | LazyComponentCtor;
    children?: Route[],
};

class MainLayout extends HTMLElement {}

declare module 'my-view.ts' {
  export class MyView extends HTMLElement {}
}

const routes: Route[] = [
    {
        path: '',
        componentClass: MainLayout,
        children: [
            {
                path: '',
                componentClass: async() => (await import('my-view.ts')).MyView
            }
        ],
    },
];

platosha avatar Apr 05 '22 13:04 platosha

Does not that require importing my-view.ts and all other views to set up routing?

Artur- avatar Apr 06 '22 11:04 Artur-

Does not that require importing my-view.ts and all other views to set up routing?

The actual script is loaded in the browser only when the callback is run, even though the direct reference to .MyView gives the impression that it's eagerly imported.

As an alternative, we could maybe expect the class to be the default export and make a componentClass callback support promises so that the line could be reduced to this: componentClass: () => import('my-view.ts')

Legioth avatar Nov 25 '22 07:11 Legioth

The import needed at the beginning of the file is also eager, right? That is currently the only solution. Maybe it would be best to change the sample code so that it fails if you try to copy-paste to add a new route?

samie avatar Nov 29 '22 09:11 samie