vaadin / flow-and-components-documentation

The documentation for the Vaadin framework version 10+
https://vaadin.com/docs/
16 stars 101 forks source link

Clarify documentation about the necessity of having a <link rel="import"> when not using @Id #844

Open thigg opened 6 years ago

thigg commented 6 years ago

Download minimal (not) working example demo.zip

This is a spring boot application using vaadin. It uses the following template:

<link rel="import" href="../bower_components/polymer/polymer-element.html">
<dom-module id="test-form">
    <template>
        <vaadin-text-field id="cake"></vaadin-text-field>
    </template>
    <script>
        class PersonForm extends Polymer.Element {
            static get is() {
                return 'test-form'
            }
        }
        customElements.define(PersonForm.is, PersonForm);
    </script>
</dom-module>

and this class:

@Route("")
@Tag("test-form")
@HtmlImport("frontend://src/TestForm.html")
public class TestView extends PolymerTemplate<TemplateModel> {
    //@Id("cake")
    //TextField tmp;
}

While the textfield is commented out, thevaadin-text-field is not rendered at all. This reason is, that the vaadin-text-field component is not transmitted to the browser in this case.

When the code is commented out: image

And when not: image

Expected Result: The component is rendered, not dependent on the binding, or this behavior is documented.

Actual Result: The component cannot be rendered, because the file for the component is not transmitted.

thigg commented 6 years ago

It works as intended when adding <link rel="import" href="../bower_components/vaadin-text-field/src/vaadin-text-field.html"> to the template, which makes sense.

It feels strange, that vaadin adds this header while the fields are bound on its own. Maybe it is enough to document this behavior and emphasize the importance of correct import definitions.

ZheSun88 commented 6 years ago

Hi, thanks for the report. and as you have figured out that the missing import causes the issue, which is not a bug.

We have made some documentation on using polymerTemplates, and you can have a look here. https://github.com/vaadin/flow-and-components-documentation/blob/master/documentation/polymer-templates/tutorial-template-basic.asciidoc .

In this case, I will close this issue. If you feel the doc needs some improvements, we are glad to get your feedback in the documentation repository

Legioth commented 6 years ago

The reason why it works this way is that when a component instance is attached to the component tree, imports are added based on @HtmlImport annotations on the component class. Thanks to this, everything just works if you'd do e.g. myLayout.addComponent(new TextField());. The same logic is also implicitly triggered when using @Id because also then there is an attached component instance with a specific class from which the imports can be discovered.

When you remove @Id, the framework has no chance of knowing what to import unless you have defined it explicitly. The application might have multiple different Component subclasses that all use the same <vaadin-text-field> tag name, so there's no way of automatically knowing what to import. It could be argued that if only one candidate class is found, then the imports from that class should be used. The problem in that case is that adding another implementation to the application's classpath could suddenly cause completely unrelated parts of the application to stop working.

thigg commented 6 years ago

Thanks for your replies and explanations. I will try to improve the documentation.

Wouldn't it be more consistent, if the @Id annotation wouldn't trigger imports?

It makes sense, that the according files are imported, when a field is created, but this case, where the binding via @Id hides missing imports, wouldn't appear.

Legioth commented 6 years ago

Wouldn't it be more consistent, if the @Id annotation wouldn't trigger imports?

That wouldn't work in cases like Grid where an additional gridConnector.js also needs to be included to make the Java API integration work. That may be worked around if it would be possible to configure whether an import annotation should be used always or only for non-@Id cases. But at that point, we're already talking about a quite complex solution.

thigg commented 6 years ago

That sounds like, adding a yellow box to the documentation Is the best we can do.