Use VariableLookup instead of dot notation in standard array filters#1899
Use VariableLookup instead of dot notation in standard array filters#1899
VariableLookup instead of dot notation in standard array filters#1899Conversation
|
Parsing filter arguments as variables at render time, with or without The first is one of efficiency. The second downside is regarding template static analysis. If we were walking a template's parse tree with I understand that you might have already considered these things and decided that the current approach is still the best option. I'm just sharing my thoughts here in the absence of an RFC 😃. |
|
Thank you for bringing that context, @jp-rp. I followed up on these thoughts at Shopify, performed additional risk assessment on the nested filters, and noticed it’s not ideal to introduce nested properties support at this time. I left some context about this here: #1907 I’m closing this PR in favor of #1906 Thanks again for those thoughts 🙇 |

Following @ianks' suggestion (https://github.com/Shopify/liquid/pull/1869/files#r1919321969), this PR updates standard array filters to use
Liquid::VariableLookupinstead of the dot notation convention.I initially considered something similar to the usage of the
Liquid::VariableLookup, but I noticed it would open the door to broader use cases like accessing arrays, which might not be ideal as mentioned here (#1869 (comment)).However, in the interest of consistency, we see value in evaluating this approach, which is the goal of this PR :)