Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request for suggestions: Add find, find_index, has, and reject filters to arrays #1849

Closed
karreiro opened this issue Nov 15, 2024 · 9 comments

Comments

@karreiro
Copy link
Contributor

Author: Shopify
Expected end date: December 5, 2024

Background

Handling arrays in Liquid is challenging.

Developers often need to iterate over arrays to find items or check for their presence. This pattern results in imperative code and verbose templates, which detract from the simplicity and declarative nature that Liquid aims to promote.

{%- liquid
  assign has_app_block = false

  for block in section.blocks
    if block.type == "@app"
      assign has_app_block = true
    endif
  endfor
-%}

{{ has_app_block }}

⏭️ Upcoming: In a different proposal, we'll tackle improvements to the where filter and its limitation of only filtering based on equality. That's so crucial, we aim to handle that separately.

Proposal

We should introduce new filters to make it easier to perform common tasks such as extracting or checking the presence of an item in an array. This will make Liquid templates simpler, less verbose, and more declarative.

All filters proposed here do not mutate the source object and return a new instance as a result of the operation:

1. find and find_index

The find and find_index filters return, respectively, the item and the index of the item with the specified property value; otherwise, they return nil.

Now Proposal
{%- liquid
  assign index = 0

  for section_block in section.blocks
    assign index = index | plus: 1

    if block.id == section_block.id
      break
    endif
  endfor
%}
{{ section.blocks | find: "id", block.id }}

{{ section.blocks | find_index: "id", block.id }}
2. has

Returns true when the array includes an item with the specified property value; otherwise, returns false.

Now Proposal
{%- liquid
  assign has_app_block = false

  for block in section.blocks
    if block.type == "@app"
      assign has_app_block = true
    endif
  endfor
-%}

{{ has_app_block }}

Source

{{ section.blocks | has: "type", "@app" }}
3. reject

Works almost like the where filter, but returns an array excluding items with the specified property value.

Now Proposal
{% assign property_value = false %}

{% if something != this %}
  {% assign property_value = true %}
{% endif %}

{{ array | where: property_name, property_value }}

Source

{{ section.blocks | reject: "type", "@app" }}

Call for suggestions

We welcome any feedback or opinions on this proposal. Please share your thoughts by December 5, 2024. Your input is valuable as we prepare to begin active development on this initiative.

@jg-rp
Copy link
Contributor

jg-rp commented Nov 15, 2024

👍

  1. find and find_index

Would some_array | find: prop, value be equivalent to some_array | where: prop, value | first, although potentially more efficient?

Have you considered find_first, find_last, find_first_index and find_last_index, to be consistent with remove_first and replace_last etc.?

@dan-gamble
Copy link

Would adding the ability to work with nested keys be a trivial add? i.e.

{{ section.blocks | find: "settings.product.id", product.id }}

@galenking
Copy link

I feel like find and find_index make sense and I like @jg-rp’s suggestion for first and last. It would be important, I think, that these are case-insensitive as case-sensitivity causes so many problems with contains (that should be another change!).

IMHO, reject makes no sense. It’s not initially intuitive and would easily be achieved with a more robust where that supports true and false statements.

@efekurnaz
Copy link

efekurnaz commented Nov 15, 2024

These all are pretty useful! An addition to boolean support idea of @galenking, comparison filters would also be really useful.

{% assign collections_to_show = collections | where_compare: 'products_count', '>=',  0 %}

or

{% assign collections_to_show = collections | greater: 'products_count',  0 %}
{% assign collections_to_show = collections | greater_or_equal: 'products_count',  0 %}

or

{% assign collections_to_show = collections | where_gt: 'products_count',  0 %}
{% assign collections_to_show = collections | where_gte: 'products_count',  0 %}

@thatecommerceguy
Copy link

{{ section.blocks | exclude: "type", "@app" }}
exclude sounds more intuitive then reject

@andershagbard
Copy link
Contributor

Pending PR for reject #1573

@jg-rp
Copy link
Contributor

jg-rp commented Nov 17, 2024

Upcoming: In a different proposal, we'll tackle improvements to the where filter and its limitation of only filtering based on equality.
That's so crucial, we aim to handle that separately.

Should where and reject/exclude be considered together? Depending on what you have in mind for where, it could influence reject syntax and semantics. Or even make it obsolete.

@karreiro
Copy link
Contributor Author

👋 Hey everyone,

Thank you so much for sharing these thoughts!


Would some_array | find: prop, value be equivalent to some_array | where: prop, value | first, although potentially more efficient?

Precisely, @jg-rp, this makes runtimes potentially more efficient and also makes the code less verbose.


Have you considered find_first, find_last, find_first_index and find_last_index, to be consistent with remove_first and replace_last etc.?

We considered it also because of the String API, but it was looking a bit verbose, and we do have some prior art (with Ruby, Rust, JavaScript) where the concept of find means returning the first element.

Regarding remove_first/replace_last, I see them a bit differently because remove/replace act on the entire string. I'd agree if we were considering a where_first filter, but find seems to be a bit closer to human language and expectation.

Regarding find_last, I've explored use cases in Shopify themes, and it feels nice but a bit niche enough to encourage folks in the array | reverse | find direction. Considering your use cases, do you feel strongly about this proposal or have any examples in mind?

Thanks a lot your thoughts, @jg-rp!


Would adding the ability to work with nested keys be a trivial add? i.e.

{{ section.blocks | find: "settings.product.id", product.id }}

[...]
Pending PR for reject #1573

Thank you, @andershagbard and @dan-gamble. This is an excellent suggestion and really resonates with so many use cases.

We're planning to incorporate this as part of the implementation. I don't think we're introducing support for multiple separators, but we may expect that on the Array API.


IMHO, reject makes no sense. It’s not initially intuitive and would easily be achieved with a more robust where that supports true and false statements.
[...]
Pending PR for reject #1573
[...]
Should where and reject/exclude be considered together? Depending on what you have in mind for where, it could influence reject syntax and semantics. Or even make it obsolete.

We've identified some contexts where the reject filter makes the code less verbose, and scenarios where it's impossible to use where. So, a negative operation for filtering really seems ideal and it's an old ask from the community #1555.

Thinking about naming, many different folks were leaning on reject as the ideal name, but we have a clear bias towards Ruby for this one. Personally, I believe that exclude may inspire a filter that returns a boolean for some users.

Please, if you consider reject not a good name, leave your thoughts here 🙇 (thanks a lot, @galenking, for raising that concern)

For sure we have some paths where a more robust where may handle reject use cases, but in the spirit of a language that supports unless, I cannot see a path where it may be obsolete.

All query-filters (reject, find, find_index, and has) will adopt the same signature as where, we're considering them together and they should all evolve together. We do have some ideas similar to what @efekurnaz proposes (which makes me think it's an intuitive path to evolve):

{{
  collections.first.products
  | where: 'price', greater: 3000
  | map: 'title'
}}

{{
  collections.first.products
  | reject: 'title', contains: "Shirt"
  | map: 'title'
}}

But we're still validating how that plays with other long-term items to identify the limitations of this approach, so we don't feel this is a point to be in an RFC for now.


Thanks again, everyone, for all your input. We read and reflect on every thought you share here and in the other issues. They are really helpful in calibrating the right direction for the language!

@karreiro
Copy link
Contributor Author

👋 Hey everyone,

Thank you so much for your thoughts here. I'm closing this issue in favor of this PR:
#1869

If you have any extra thoughts, feel free to drop them there. This way, we can keep a single thread about this :)

Thanks again for your involvement!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants