fix(theme-language-server-common): preserve element type on array-returning filters#1184
Open
Jp-fix wants to merge 1 commit into
Open
fix(theme-language-server-common): preserve element type on array-returning filters#1184Jp-fix wants to merge 1 commit into
Jp-fix wants to merge 1 commit into
Conversation
Author
|
I have signed the CLA. |
aswamy
reviewed
May 15, 2026
Contributor
aswamy
left a comment
aswamy
left a comment
There was a problem hiding this comment.
Thanks for the contributions Jp!
I've left a few suggestions in the PR. Also, please follow the PR template for your PR descriptions
https://github.com/Shopify/theme-tools/blob/main/.github/PULL_REQUEST_TEMPLATE.md
| '@shopify/theme-language-server-common': patch | ||
| --- | ||
|
|
||
| Preserve the element type when piping a typed value through `sort`, `sort_natural`, `reverse`, `uniq`, `compact` or `where`. Previously these filters collapsed the type to `untyped[]` because their `array_value` is declared as `untyped` in the filter docs; they are now treated as pass-through in the type system. Fixes #1086. |
Contributor
There was a problem hiding this comment.
Keep the changeset short and sweet. No implementation details or ticket numbers.
Comment on lines
+607
to
+624
| /** | ||
| * Filters whose return type is an array of the same element type as their | ||
| * input. The filter docs in `filters.json` declare `array_value: "untyped"` | ||
| * for these, which would make every pipeline collapse to `untyped[]`. We | ||
| * special-case them here to preserve the element type of the input. | ||
| * | ||
| * Intentionally excluded: | ||
| * - `map` — projects a property, needs lookup resolution on the element type. | ||
| * - `concat` — merges two arrays, element type would be a union. | ||
| */ | ||
| const ARRAY_PASSTHROUGH_FILTERS: ReadonlySet<string> = new Set([ | ||
| 'sort', | ||
| 'sort_natural', | ||
| 'reverse', | ||
| 'uniq', | ||
| 'compact', | ||
| 'where', | ||
| ]); |
Contributor
There was a problem hiding this comment.
Instead of having a static allow-list, could we invert this and exclude all array filters that isnt map or concat.
| 'where', | ||
| ]); | ||
|
|
||
| function elementTypeOf(t: PseudoType | ArrayType): PseudoType { |
Contributor
There was a problem hiding this comment.
Have a comment that mentions that non-array inputs are treated as a single-element array of that type
Suggested change
| function elementTypeOf(t: PseudoType | ArrayType): PseudoType { | |
| /* Returns the element type of an array, or the input itself if it is a scalar. */ | |
| function elementTypeOf(t: PseudoType | ArrayType): PseudoType { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What are you adding in this PR?
Fixes #1086.
When a typed value is piped through an array-returning filter like
sort, the language server was showinguntyped[]on hover instead of preserving the element type. Example from the issue:{% for media in product.media %} {% assign media_as_array = media | sort %} {# hover on `media_as_array` used to show `untyped[]` — now shows `media[]` #} {% endfor %}The filter docs declare
array_value: "untyped"for these filters, sodocsetEntryReturnTypewas always handing backuntyped[]. Instead of changing the docs (they're pulled fromShopify/theme-liquid-docs), I added a small pass-through list inTypeSystem.tscoveringsort,sort_natural,reverse,uniq,compactandwhere, and switched the filter chain from "only inspect the last filter" to a left-to-right reduce so chains likex | sort | reversethread the element type correctly.Scalar inputs (
media | sortin the issue) are wrapped into an array of the same type, matching Liquid's runtime coercion.What's next? Any followup issues?
Before you deploy