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: Make it easier to append and prepend items in arrays #1851

Open
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 convert arrays into strings, perform operations, and then split them back, relying heavily on the Liquid String API as it's more robust than the Array API.

{% assign titles = collections.last.products | map: "title" %}

{{
  titles
  | join: ","
  | append: ",default"
  | split: ","
}}
{{
  titles
  | join: ","
  | prepend: "All,"
  | split: ","
}}

Proposal

We should eliminate workarounds like string concatenation and splitting and make it easier to perform common tasks such as adding new items in an array. This will make Liquid templates simpler, less verbose, and more declarative.

To make that happen, we should support the append and prepend filters in arrays. They will return a new array with the specified element added to the end or beginning, respectively.

{% assign titles = collections.last.products | map: "title" %}

{{ titles | append: "default" }}
{{ titles | prepend: "All" }}

Limitations

This proposal is not backward compatible [1] [2].

{{ titles | append: "new" }}
{% # => "[\"Example T-Shirt\", \"Classic Varsity Top\", \"Classic Leather Jacket\", \"Dark Denim Top\"]new" %}

Therefore, the implementation is pending an impact assessment. If we identify it's impossible to handle backward compatibility, we aim to introduce only the append functionality instead, using an unused filter name (e.g. add).

We're proposing the append/prepend direction first because we consider this a more idiomatic and cohesive direction for the Liquid API.

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.

@galenking
Copy link

Would lack of backwards-compatibility not break any existing theme that’s treating append as a string? Will there be any way to determine which version of liquid a theme uses?

Would it not make more sense to use push and unshift? Although, I personally find prepend and append more intuitive and I do think it’s a better pairing for the next generation of devs.

@TeamDijon
Copy link

TeamDijon commented Nov 15, 2024

Just so you know, you can use the following syntax to append elements in an array:

{% liquid
  assign string_list = "second,third,fourth" | split: ','
  assign string_to_prepend = "first"
  assign string_to_append = "fifth"

  # Prepending to a Liquid array
  assign string_list = string_to_prepend | uniq | concat: string_list

  # Appending to a Liquid array
  assign string_list = string_list | reverse
  assign string_list = string_to_append | uniq | concat: string_list | reverse

  echo string_list | append: '<br>'
  # => ["first", "second", "third", "fourth", "fifth"]
%}

Sure, dedicated filters would be nice but I wouldn't call Array API inconsistent or less robust. It is simply under-explored by most Shopify developers

I agree with @galenking, push and unshift filters would make more sense. No need to bring confusion with different behaviors according to the data type it is used on. Dedicated array filters, whatever the name would be better in my opinion !

@jg-rp
Copy link
Contributor

jg-rp commented Nov 15, 2024

Would it not make more sense to use push and unshift?

I think I prefer push and push_left.

@kinngh
Copy link

kinngh commented Nov 15, 2024

append and prepend are easy to confuse with string concatenation. Keeping it to a simple push would do a great job.

Adding element to the end of array:

{{ titles | push: 'new_title' }}

Adding element at the beginning of an array:

{{ 'new_title' | push: titles }}

Edit: Since we already know titles is an array, it doesn't make sense to reassign it.
Edit 2: Swapped append with push

@wbelk
Copy link

wbelk commented Nov 15, 2024

+1 on arrays

better support for an array type

{% assign foo = [ 'c' ] %}
# foo: ['c']

{% push foo = 'a' %}
# foo: ['c', 'a']

{% unshift foo = 'b' %}
# foo: ['b', 'c', 'a']

{{ foo | pop }}
# 'a'

{{ foo | shift }}
# 'b'

@sadsciencee
Copy link

would love better array support. using existing keywords doesnt make sense to me. the functionality is different, a different keyword makes sense. plus as mentioned backwards compatibility issues would cause massive problems for custom dev shops

@karreiro
Copy link
Contributor Author

👋 Hey everyone,

Thanks for the feedback!

One constraint we have is that we need to keep the original array immutable in Liquid.

Thus, we did consider the pop, push, shift, and unshift names. However, all these concepts are generally associated with array mutation in other languages, so we may break developers' expectations by not mutating them.

Currently, we're considering prepend/append as good options, to be cohesive with the String API. But, if other folks consider that unexpected or a bit confusing as @kinngh mentioned, please share your thoughts 🙇

Maybe the assumption that prepend/append are expressive names is wrong (in that case, we don't need to even assess the impact of such a path). We also considered names closer to human language such as prefix and affix, and also terms closer to programmer jargon like insert.

Thanks again, everyone, for the feedback!

@jg-rp
Copy link
Contributor

jg-rp commented Nov 18, 2024

we did consider the pop, push, shift, and unshift names

I agree, prepend and append would be better than push etc., if you can accept the behavioural change. Although append might still imply a mutating operation to some (Ruby's Array.append is an alias for Array.push, Python's List.append mutates the list).

Modifying the plus filter to handle arrays might better convey the fact that you're getting a new array, but I'm not convinced it is the best choice.

Edit: I've just seen you've already mentioned using add. 👍 I should have ready the proposal more carefully.

@MatthewRCrigger
Copy link

A little late to the party, and honestly I don't have anything groundbreaking or insightful to offer that everyone else has.

I am a big user of the app DataJet, and they fork Liquid and have added some really easy to use filters and tags I am massively fond of. Maybe instead adding more functionality to the existing filters which could cause backwards compatibility issues, maybe a new tag could be used?

I don't know if their documentation and examples help with any inspiration or if it's just wasting time to even look at, but I'm just throwing them out into the wind: https://docs.datajet-app.com/liquid/tags/push

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

8 participants