Qute: fragments UX issues

[FroMage]

Description

#28476 introduced type-safe fragments, with the following syntax on the Java side:

    @CheckedTemplate
    public static class Templates {
        public static native TemplateInstance items(List<Item> items);

        @CheckedFragment
        static native TemplateInstance items$price(Item item);

        @CheckedFragment
        static native TemplateInstance items$priceEdit(Item item);
    }

The semantics is that a method named foo$bar refers to the fragment with identifier bar defined in the foo.html file. The $ sign is used as separator to distinguish the template file and the identifier. This is required because using simply the foo part as the method name would introduce a clash with the containing template declaration, and using only the bar part could also clash with a similarly named method defined for a template called bar.html (even if we distinguished it with @CheckedFragment(inside = "foo")).

In my opinion, the presence of $, being required due to the above, is enough to denote that we're talking about a fragment, and thus the @CheckedFragment annotation is boilerplate and should be removed. @mkouba argues that it's more explicit, but IMO the $ stands out enough by itself.

Now, on the other side of things, we have the following syntax for declaring fragments:

{#fragment id=price}
 something
{/fragment}

I think it's a mistake to denote the ID as something that looks like a symbol, because it's not consistent with other Qute expressions. Indeed, the Qute reference doesn't even mention symbols as a possible literal: https://quarkus.io/guides/qute-reference#literals so this looks like we're using the price variable as ID, which is very confusing. As such, I think the ID should be defined as a String value:

{#fragment id='price'}
 something
{/fragment}

Implementation ideas

No response

[quarkus-bot]

/cc @mkouba

[ia3andy]

I agree with the above.

I would add that id is also something from html tags which could be confusing. Why not:

{#fragment price}
 something
{/fragment}

Also for the $ reference, @mkouba mentioned a # to use it in an include (can't we be consistent?)

[maxandersen]

@ia3andy Wouldn’t use of id be consistent with html?

I would lean to use “” string for the id rather than what looks to varible reference.

I would say I lean to agree that if we require BOTH to name something with $ AND add @checkedfragment and it is not possible today to use a variable with $ for data then $ should be sufficient.

[mkouba]

@CheckedFragment

I explained my arguments in the comments of #28476, here and here. But I can repeat it here. The annotation:

1. makes it safe because in theory a template with the $ sign in name may already exist
2. makes it more readable and more apparent - $ in the method name can be easily overlooked and even if spotted requires the user to read the docs in order to find out what it's about. In contrast, an annotation is very clear, has javadoc, etc.

I'm -1 and unless you bring a more reasonable argument I'm not going to change my mind.

Fragment id

In fact, {#fragment price}, {#fragment id=price} and {#fragment id='price'} all of these are supported and tested. The id is an optional name of a parameter and the value is never evaluated, i.e. it's not an expression.

I think it's a mistake to denote the ID as something that looks like a symbol, because it's not consistent with other Qute expressions.

Qute sections work a bit differently compared to expressions. A specific SectionHelperFactory is responsible to handle the parameters when a template is parsed. In fact, the section factory sees the params as Map<String, String> and it can decide to parse some of the parameters as expressions. You may not like the API but we use since v1.0.

Now we use the same strategy for the {#include} section for a long time already.

$ vs #

Also for the $ reference, @mkouba mentioned a # to use it in an include (can't we be consistent?)

I was actually wrong we use a different syntax in the {#include} section: {#include items[price] /} or even {#include [price] /} if you need to reference a fragment from the current template.

[FroMage]

makes it safe because in theory a template with the $ sign in name may already exist

This is a theoretical argument at best. We don't want to support that, and the resulting template method would look like what, items$foo$bar? Why would people want to do something like that? We need to place limits as to what's reasonable.

makes it more readable and more apparent - $ in the method name can be easily overlooked and even if spotted requires the user to read the docs in order to find out what it's about. In contrast, an annotation is very clear, has javadoc, etc.

I really don't see how a $ can be overlooked. But remember, people from Spring are used to method names being magical and specify entire HQL queries. People from other frameworks are used to methods starting with get/put etc being synonymous with @GET/@PUT. And in our case, we already have @CheckedTemplate on the class, which makes this class very special, and those annoying static native method modifiers. Plus fragments are an advanced feature, I don't see people using fragments being confused as to whether the method they just typed with a $ sign is a fragment or not.

And also, what purpose or confusion could arise from removing the annotation? The typechecker is always going to validate that the template fragment exists when there's a $ sign. I don't see what possible error situation the lack of an annotation could cause to the user. From the template fragment caller's PoV, this invokes like a template, and returns a template, and it doesn't make any difference. I see exactly zero benefit to the annotation, not in terms of visibility, or type safety, or even IDE support.

In fact, {#fragment price}, {#fragment id=price} and {#fragment id='price'} all of these are supported and tested. The id is an optional name of a parameter and the value is never evaluated, i.e. it's not an expression.

OK, but that's my point: it's confusing to users that they can't figure out exactly what is an expression or not. It's much more regular if users can rely on knowing that there are statements and expressions, and that expressions are strings, numbers, etc. So I'd advise sticking to strings for includes and IDs. It's funny that you're super strict when it comes to annotations on fragments, but super lax when it comes to what expressions are in Qute ;)

I was actually wrong we use a different syntax in the {#include} section: {#include items[price] /} or even {#include [price] /} if you need to reference a fragment from the current template.

OK, so this looks even more confusing IMO, because it's very likely that we'll have a template parameter called items in the items.html file that defines this price fragment. So items[price] looks to me like extra confusing and likely to clash with the template parameter. Now, I know it's not an expression, so it doesn't clash. But I'm saying it's confusing to users where things are expressions and things are not.

If it's not an expression, then {#include 'items[price]'/} or {#include 'items.price'/} or {#include 'items#price'/} (which is still validated) sounds more reasonable. Or we introduce a make-pretend syntax, like {#include templates:items[price]/} which makes it look like we have a map of every fragment of every template defined in the templates: namespace.

[maxandersen]

on @CheckedFragment being always required because there potentially could be a user having $ in their filenames...how about flipping it around and say that for those users that has that exceptional case they can add an annotation (i.e. @CheckedIdentifier or similar). Making the exception what you absolutely need an annotation for ?

[ia3andy]

and why not {#include 'items$price'/}?

[mkouba]

This is a theoretical argument at best. We don't want to support that, and the resulting template method would look like what, items$foo$bar? Why would people want to do something like that? We need to place limits as to what's reasonable.

Still a valid argument. We do not forbid using any character in the method name. Which implies that any valid java method name is OK. I don't think we should break compatibility unless we really must.

So I'd advise sticking to strings for includes and IDs.

Fair enough. It's a user choice...

It's funny that you're super strict when it comes to annotations on fragments, but super lax when it comes to what expressions are in Qute ;)

@FroMage I don't think I'm super lax here ;-). It's well defined. There are standalone expressions and there are sections that may treat params as expressions. Validation and interpretation of the parms is the business of a particular section. This allows for custom "rules" such as:

• {#for item in items} - neither item nor in are expressions
• {#if item.active or item.sold} - or is not an expression
• {#when machine.status}{#is ON}It's on!{/when} - ON is not an expression if machine.status evaluates to an enum
• ...

If it's not an expression, then {#include 'items[price]'/} or {#include 'items.price'/} or {#include 'items#price'/} (which is still validated) sounds more reasonable.

{#include 'items[price]'/} should work just fine. TBH I don't remember why I decided to use the square brackets in the introductory pull request... maybe I wanted to use something that would unlikely crash existing templates. Maybe I just copied syntax from some other templating engine 🤷 .

It's important to note that a fragment id can only match [a-zA-Z0-9_]+. So {#include items#price /} or {#include items$price /} could work as well. We could also introduce a special include section for fragments. But that's probably not very convenient.

[mkouba]

on @CheckedFragment being always required because there potentially could be a user having $ in their filenames...how about flipping it around and say that for those users that has that exceptional case they can add an annotation (i.e. @CheckedIdentifier or similar). Making the exception what you absolutely need an annotation for ?

That's an interesting idea... actually, we don't need a new annotation. We could just add a safety switch to the @CheckedTemplate.

[mkouba]

Something like: @CheckedTemplate#ignoreFragments(). By default false, if set to true then quarkus will not attempt to validate a checked fragment for a method with a name that contains $.

And we could use the same strategy for the {#include} section, i.e. {#include items$price /} or {#include 'items$price' /}, and {#include 'items$price' ignoreFragments=true /}.

@maxandersen @FroMage @ia3andy ^ WDYT about this compromise?

[ia3andy]

Something like: @CheckedTemplate#ignoreFragments(). By default false, if set to true then quarkus will not attempt to validate a checked fragment for a method with a name that contains $.

And we could use the same strategy for the {#include} section, i.e. {#include items$price /} or {#include 'items$price' /}, and {#include 'items$price' ignoreFragments=true /}.

@maxandersen @FroMage @ia3andy ^ WDYT about this compromise?

Well it seems to solve it all at once :)

[FroMage]

@FroMage I don't think I'm super lax here ;-). It's well defined. There are standalone expressions and there are sections that may treat params as expressions. Validation and interpretation of the parms is the business of a particular section. This allows for custom "rules" such as:

• {#for item in items} - neither item nor in are expressions
• {#if item.active or item.sold} - or is not an expression
• {#when machine.status}{#is ON}It's on!{/when} - ON is not an expression if machine.status evaluates to an enum

Well, item is a symbol, it's a declaration of a new variable. or is a symbol, it's an alias for the || operator, and if Qute had a regular grammar, the triplet a or b would be an expression equivalent to (a || b) and not three different arguments to if being interpreted according to special if rules ;). And ON is also a symbol: it's a reference to a declared variable. All these actually fall under the regular syntax for expression evaluation.

Now, perhaps you are implying that IDs should be symbols, and thus be declared and used without quotes, which means they have their own namespace, separate for that of expressions. Like in Java methods, fields and classes have three separate namespaces?

[FroMage]

@maxandersen @FroMage @ia3andy ^ WDYT about this compromise?

Which means we can get rid of @CheckedFragment? :)

[mkouba]

@maxandersen @FroMage @ia3andy ^ WDYT about this compromise?

Which means we can get rid of @CheckedFragment? :)

Am I not clear enough? :D

PS. Yes, because we'll have @CheckedTemplate#ignoreFragments() as a safety switch.

[FroMage]

Well, great then! :)

[mkouba]

and if Qute had a regular grammar

It doesn't atm.

[FroMage]

Well, I know, but we should still add features which aim to emulate this and keep it possible to add/define such a regular grammar at some point.