vaadin / hilla

Build better business applications, faster. No more juggling REST endpoints or deciphering GraphQL queries. Hilla seamlessly connects Spring Boot and React to accelerate application development.
https://hilla.dev
Apache License 2.0
895 stars 56 forks source link

Hilla forms reference documentation examples no longer works correctly #843

Open ggecy opened 1 year ago

ggecy commented 1 year ago

Vote to prioritize: add a 👍 reaction to help the community and maintainers prioritize this issue.

On https://hilla.dev/docs/data-binding/reference page and others in same section you define model properties in examples as readonly properties, e.g.:

readonly fullName = new StringModel(this, 'fullName', new NotEmpty({message: 'Cannot be empty'}));

If the model properties are not defined as getters using this[_getPropertyModel]() inside, e.g.:

get amount() {
    return this[_getPropertyModel]("amount", NumberModel, [false]);
}

then @hilla/form validation no longer works correctly since it explicitly checks for getters in model. Btw this is a breaking change which was introduced in minor version update in @hilla/form which is wrong...

I know that your intent is to generate the models from endpoints, but on our projects we cannot use the hilla server side endpoints because of incompatible security model so we have to define the client models manually and the documentation was very misleading in this case.

jouni commented 1 year ago

Not sure if this matters, but that documentation page might be outdated, as the currently maintained one is here: https://hilla.dev/docs/lit/guides/forms/reference

We have an issue in our deployment script that it does not delete the previous deployment before publishing a new one. It only overwrites existing files (and creates new ones). I’ll ping our admin again about this. This issue has been resolved.

ggecy commented 1 year ago

Not sure if this matters, but that documentation page might be outdated, as the currently maintained one is here: https://hilla.dev/docs/lit/guides/forms/reference

We have an issue in our deployment script that it does not delete the previous deployment before publishing a new one. It only overwrites existing files (and creates new ones). I’ll ping our admin again about this.

Well, even this one uses the wrong examples of model classes

readonly idString = new StringModel(this, 'idString');
platosha commented 1 year ago

Sounds like a bug we should fix. Let us add support for fields on par with getters.

Note: with fields it is impossible to define a self-referencing model. That's why we moved to using getters in generated models.

ggecy commented 1 year ago

Sounds like a bug we should fix. Let us add support for fields on par with getters.

Note: with fields it is impossible to define a self-referencing model. That's why we moved to using getters in generated models.

It's not a problem to use the getter notation (maybe a little bit more user friendly notation would be welcome), just that the documentation has old examples which no longer work properly.