Add find, find_index, has, and reject filters to arrays

[karreiro]

This PR implements the RFC#1864 and introduces support for the find, find_index, has, and reject filters into the Liquid Array API.

{% assign product = products | find: 'price_unit', 'BRL' %}

For the sake of consistency, multi-level access has been introduced for all related filters in the Array API: map, sort, sort_natural, sum, uniq, where, and compact.

--

Special thanks to @andershagbard who started this work on #1573 and #1749. I'm including those commits as part of this PR; while the implementation approach is slightly different, important work happened there, and it's valuable to have that context in the Liquid commit history.

Also, thank you to everyone who contributed with thoughts and suggestions in the RFC.

[karreiro]

👋 Hey @charlespwd @Maaarcocr,

Could you please take a look at this PR?

Thank you!

[charlespwd]

LGTM :)

[bakura10]

Awesome news <3. That's a very welcome addition to Liquid which will help us to clean up a lot of our code. Do you know when this will be available to use in Shopify?

[ianks]

This seems weird to me... does this mean that:

product.dafsdfasdfasdf #=> ProductDrop

I would have expected it to be like:

product.dafsdfasdfasdf #=> nil

[karreiro]

That would be weird indeed. But I believe it's already working as expected (and product.dafsdfasdfasdf already returns nil), as this test passes:

def test_prop_test
  products = [
    { "title" => "Pro goggles" },
    { "title" => "Thermal gloves", "price" => 1499 },
    { "title" => "Alpine jacket",  "price" => 3999 },
    { "title" => "Mountain boots", "price" => nil },
    { "title" => "Safety helmet",  "price" => 1999 },
  ]

  template = <<~LIQUID
    {%- assign example1 = products | find_index: 'dafsdfasdfasdf' -%}
    {{- example1 -}}
    {%- assign example2 = products | find_index: 'dafsdfasdfasdf.dafsdfasdfasdf' -%}
    {{- example2 -}}
  LIQUID
  expected_output = ""

  assert_template_result(expected_output, template, { "products" => products })
end

The reason we're achieving the expected behavior is that:

• For example1, we're resolving that at line 985
• For example2, we're resolving that because in the first iteration drop is product and returns nil for the first dafsdfasdfasdf call, then in the second call drop is already nil

So, I believe we're already achieving that behavior, but please let me know if I'm missing something :)

[ianks]

I'm not sure how I feel about re-implementing lookup logic here. There's already a way to do that which is well-defined with VariableLookup.parse. It even has built-in caching to mitigate performance issues of repeated parsing. Could we use this instead?

irb(main):006> Liquid::VariableLookup.parse("product.images[0]")
=> #<Liquid::VariableLookup:0x000000010565ce98 @command_flags=0, @lookups=["images", 0], @name="product">

[jg-rp]

I have reservations about parsing strings at render time in general. At the very least it will play havoc with static analysis.

[karreiro]

Let's continue this thread here:
#1899 :)

[karreiro]

👋 Hey everyone,

We’ve conducted additional assessments of the backward-compatibility of this change and confirmed this is not 100% backward-compatible as stated here.

We’re keeping the new filters, but the nested properties will be re-assessed and implemented by #1906.

I have valuable concerns raised by the community and powerful alternatives that deserve deeper consideration.

Nested properties need to be supported, but they won’t be part of 5.7 (#1907), as it’s more involved than this PR states.

Thanks to everyone following this PR.

[rickydazla]

Love this update! Usage question: how would I use check if product.tags contains a certain value?

product.tags | has: '???', 'MyTag'

Using any of title, name, tag, or tags in the above doesn't seem to work...

[andershagbard]

Love this update! Usage question: how would I use check if product.tags contains a certain value?

product.tags | has: '???', 'MyTag'

Using any of title, name, tag, or tags in the above doesn't seem to work...

https://shopify.dev/docs/api/liquid/basics#contains

if product.tags contains 'MyTag'

[bakura10]

@karreiro , after testing this feature, I was a bit surprised that find_index and has does not allow to be used without properties. Would it be possible to support things like that:

{% assign has_product = block.settings.product_list | has: product %}
{% assign product_index = block.settings.product_list | find_index: product %}

[andershagbard]

@karreiro , after testing this feature, I was a bit surprised that find_index and has does not allow to be used without properties. Would it be possible to support things like that:

{% assign has_product = block.settings.product_list | has: product %}
{% assign product_index = block.settings.product_list | find_index: product %}

I think that the product_list input is a bit weird. It doesn't seem like an actual array when accessing it's properties.

But, it would be nice if these filters could handle objects as well, like contains do.

{%- liquid
assign variant = product.variants.first

# false
product.variants | has: variant

# blank
product.variants | find_index: variant

# true
if product.variants contains variant