handle .length and .items quirks by tagging uses with an __njk signature which is specially handled

[risicle]

https://trello.com/c/x2jjzXJS

Custom implementations of the Environment's getitem and getattr are used to detect these and attempt to take the best action when either of these are used. We limit this to just lookups with an __njk signature because we don't want to alter the behaviour of all of an app's jinja, and unfortunately it's not possible to switch Environments as jinja enters into an .njk file. So we need to specially tag those .length and .items accesses that originated from an njk file in the preprocess step.

This is the only way I could think of reliably fixing test_date_input_with_default_items. The issue with this test is how components/date-input/template.njk assigns the result of the params.items lookup to a variable and then iterates over that later. This way of handling these lookups seems to me like it would be much more reliable than pure string replacement.

[lfdebrux]

So if I understand correctly the bug is caused by Jinja return the items attribute in response to a lookup for a dictionary item called items? Is this a known bug upstream?

[risicle]

No, I think this arises when there is no items dict entry to find. The lookup then falls back to finding the attribute method items. Reading the source, I gather that the difference between ['foo'] and .foo in jinja is just which will be prioritized - attrs or dict entries. It will still fall back to the other. So taking that into account, this isn't really a bug per se.

Though really, I just see this as a more robust way of approaching the items and length differences rather than just going after specific bugs.

[lfdebrux]

I see.

I would prefer to implement this by using standard Jinja features rather than overriding it's behaviour; I think that leaves us open to the ground changing underneath us, which would be especially problematic if we need to support multiple versions of Jinja in production.

Jinja has a filter called attr() which only gets attributes... Could we make something like that for items?

[risicle]

Well the way I see it the Environment.getitem and Environment.getattr methods are specifically there as hooks to allow these behaviours to be customized, and here we take steps to try and make sure the behaviour only changes for the njk case.

In general, I'm made a bit nervous by the whole string-replacement approach and the ways in which it could go wrong, potentially outputting unsafe strings or data that wasn't meant to be output at all. But :man_shrugging: up to you really.