Qute: prefix loop metadata

[FroMage]

Description

https://quarkus.io/guides/qute-reference#loop_section says that we get iterator metadata index, hasNext, odd and even, but it does not show any example of how to access it, leading me to believe it would be a property of the iterated variable such as it.index. I now realise it showed this example:

{#each items}
  {count}. {it.name} 
{/each}

Which introduces the count metadata, I guess, but because it's not properly introduced I assumed it was a random toplevel.

The docs should be more clear as to which metadata properties are available, and also this is terrible wrt scoping, because it will automatically override any explicitely declared binding named count or index by surprise.

I do agree it'd be weird to tack these metadata properties to the iterated variable.

In other templates, I've seen the iterated variable name being used as a prefix, though, such as it_count or myVar_hasNext which greatly limits binding clashes.

Could we get such prefixed properties instead?

Implementation ideas

No response

[quarkus-bot]

/cc @mkouba

[mkouba]

The docs should be more clear as to which metadata properties are available,

I agree that the docs should be improved.

also this is terrible wrt scoping, because it will automatically override any explicitely declared binding named count or index by surprise.

I don't think it's a big problem. It will never override a property of an iterated element which is what matters in 95% of cases. It can override some template data or a scope variable defined by {#let} but I believe that the number of such cases is minimal.

I do agree it'd be weird to tack these metadata properties to the iterated variable.

Yes, that would make no sense.

In other templates, I've seen the iterated variable name being used as a prefix, though, such as it_count or myVar_hasNext which greatly limits binding clashes. Could we get such prefixed properties instead?

Well, it would reduce the probability of a collision but a name prefixed with it_ or myVar_ still does not make a lot of sense IMO. We could use the underscore as the prefix, e.g. {_index} and {_hasNext}. That should be good enough. However, we would have to support both the prefixed and unprefixed aliases for some time due to backward compatibility.

[mkouba]

Hm, the more I think about this, the more I think it's not worth the effort. I mean we should definitely introduce the metadata properly in the docs (right now there's only one example and a TIP that lists all properties). But I'm not so sure about the change. We could make it configurable, globally or per loop, but I doubt that someone will ever use it. We could introduce a prefixed version but then we would have to keep the old version for backward compatibility and so we would still override the variables from the outer scope.

[Ladicek]

You could add a syntax to declare a new "object" that would contain the metadata. Something like:

{#for item, meta in items}
  ... use {meta.index} ...
  ... or {meta.even} ...
{/for}

I don't know about {#each}, that feels like a completely needless syntax sugar :-)

[mkouba]

You could add a syntax to declare a new "object" that would contain the metadata.

Yes, I've been thinking about something like {#for item in items with meta} but we can't just remove the current behavior and "per loop config" is probably "needless complication" ;-).

I don't know about {#each}, that feels like a completely needless syntax sugar :-)

Yeah, "needless syntax sugar" everywhere! I personally like the {#each} form, it's short and concise and it's very explicit about the functionality it provides.

[Ladicek]

Choosing where to stop with adding syntax sugar is certainly one of the more artistic parts of language design :-)

[mkouba]

PR to improve the docs: #20687

[FroMage]

Honestly, IMO the best bet is to prefix it with the iterated variable name, because it greatly reduces shadowing issues (index and count are really too generic, there can be lots of templates where I expect those variables to already exist), and it makes it possible to nest them:

{#for f in foos}
 {#for g in f.gees}
  {f_index}/{g_index}: {g}
 {/for}
{/for}

[mkouba]

Ok, we could easily implement this but like I said we'll have to keep the original approach for backwards compatibility, or at least provide some way of switching back to the old behavior...

[FroMage]

I'm not entirely sure it's worth the effort, but is there any way to keep the old behaviour at the same time as the new, and issue a warning for deprecation if we detect someone using the old behaviour?

[mkouba]

but is there any way to keep the old behaviour at the same time as the new, and issue a warning for deprecation if we detect someone using the old behaviour?

We can log a warning whenever the old-style is used at runtime but that could be really annoying. I don't think there's an easy and reliable way to detect the problem when analyzing the templates, e.g. during build.

[FroMage]

Then perhaps yes, a new property to enable the old behaviour, defaulting to false? I don't know how many people use this.

[mkouba]

Then perhaps yes, a new property to enable the old behaviour, defaulting to false?

Ok, let's make it configurable. It will be a breaking change so we'll target 2.5.

I don't know how many people use this.

I believe that it's a commonly used functionality...

[FroMage]

OK thanks!

[gsmet]

Please make sure we have a very actionable error message so that people know how to fix it. I'm pretty sure it's often used, given how often you have to special case the first or last iteration.

[mkouba]

While implementing the change I ran into a problem: we're not able to detect an invalid usage of iteration metadata keys at runtime without false positives. For example, if a template looks like {#each items}{count}{/each} and the config will say that the iteration alias should be used as the prefix (i.e. {it_count}) then we're not able to say that {count} is actually an invalid usage. We can log a warning and it should be valid in most cases but definitely not 100%.

The truth is that for type-safe templates with @CheckedTemplate#requireTypeSafeExpressions=true (default) an invalid usage of iteration metadata keys should result in a build time error. And we could improve the error message so that it's clear what happened... But not for other types of usage.