vaadin / flow

Vaadin Flow is a Java framework binding Vaadin web components to Java. This is part of Vaadin 10+.
Apache License 2.0
603 stars 165 forks source link

Document the need to import JavaScript when TemplateRenderer renders Web Components #6265

Open mvysny opened 5 years ago

mvysny commented 5 years ago

Say that I have a TemplateRenderer in a Grid as follows:

TemplateRenderer.of<Item>("<vaadin-menu-bar items=[[item.menuItems]] on-item-selected='handleMenuItemSelect'></vaadin-menu-bar>")

If the project never used the MenuBar component before, the necessary JavaScript files/modules will not be imported and nothing will be rendered in the Grid (well, the <vaadin-menu-bar/> element will be in the DOM but it will be "dead").

The TemplateRenderer javadoc (and the TemplateRenderer documentation at https://vaadin.com/docs/v14/index.html ) should hint the need of importing all web components used manually. In case of the MenuBar, all the necessary JS files can be imported as follows:

UI.getCurrent().getInternals().addComponentDependencies(MenuBar.class);

Vaadin 14.0.0.

mvysny commented 5 years ago

skeleton-starter-flow.zip

Example project. Upon running, it will render an empty grid. However, if you uncomment the ...getInternals()... line in the sources, everything will start working properly.

mvysny commented 5 years ago

There in fact is an annotation that performs just that: the @Uses annotation, however it probably can't be used with the TemplateRenderer.

pleku commented 5 years ago

Instead of suggesting to use any getInternals() code, the javadocs for TemplateRenderer should suggest to use @JsModule instead. @Uses would be great, but probably it is not checked in this context.

mvysny commented 5 years ago

I don't think @JsModule is a good suggestion. For example the versions may change in newer Vaadin releases, which would require the user to rewrite all @JsModules in all custom TemplateRenderers the user defined.

Also, I don't think it's possible to add annotation to a code like this: TemplateRenderer.of("<vaadin-button></vaadin-button>");

pleku commented 5 years ago

I'm not sure if I understand what you mean with the version change, since @JsModule does not have any version in it. And I meant that the annotation should be added to the component class (view or custom component) which uses the template renderer, then it will be picked up.

The biggest issue is with the fact that probably @Uses should work or there should be a "public" API for making sure the web components from template renderer html are picked up. I suspect that technically the idea of trying to scan and detect the used web components from the template string is not feasible. But couple of extra things to note are:

1) TemplateRenderer will be dropped at some point, at least in the form it is supported currently once we implement LitHTML support and drop Polymer. That won't happen for Vaadin 14 2) we would have to switch this issue to an enhancement instead to make be able to add a new feature for handling this case in Vaadin 14.

So to properly fix this, it should be considered as an enhancement for 14.

mvysny commented 5 years ago

@JsModule does not have any version in it

Sorry, my bad, I've confused this with @NpmPackage. Please disregard that particular comment.

Switching this to enhancement sounds good. To implement this really quickly, we can do two things:

  1. Add TemplateRenderer.uses(Class<? extends Component>... components) fluent API which calls internals;
  2. Mention the need to call this function in the TemplateRenderer javadoc.

This would be "good enough", and not too much work (since TemplateRenderer will be dropped anyway as you said).