Fix regression when using empty/nil properties with array filters#1944
Merged
Fix regression when using empty/nil properties with array filters#1944
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a regression with array filters ensuring that empty strings and nil properties act as no-ops rather than producing errors. Key changes include:
- Updated tests for the map and where filters to verify that nil and empty string properties return the original array unchanged.
- Adjusted the map filter and the internal filter_array helper in the standard filters to handle empty properties as no-ops.
- Bumped the library version from 5.8.2 to 5.8.3.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/integration/standard_filter_test.rb | Updated tests to assert no-op behavior when array filters are given nil/empty property values. |
| lib/liquid/version.rb | Updated version to reflect the patch release. |
| lib/liquid/standardfilters.rb | Modified map filter to immediately return the input for an empty property, and adjusted filter_array to remove an early return for empty properties. |
Comments suppressed due to low confidence (2)
lib/liquid/standardfilters.rb:995
- Ensure that the removal of the early return for empty properties in filter_array is intentional; consider adding an inline comment to clarify that an empty property is now handled as a no-op to align with the updated behavior in other filters.
property = Utils.to_s(property)
test/integration/standard_filter_test.rb:588
- Consider adding test cases for the sum filter (which uses filter_array) with nil or empty properties to ensure consistent no-op behavior across all array filters.
assert_equal(input.flatten, result)
Contributor
Author
|
Verified this against shadowed traffic and things look fine, this should be good to go |
dpetrick
approved these changes
Apr 4, 2025
d35f47d to
19e287d
Compare
This was referenced Apr 13, 2025
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
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.
Fixed a regression where empty properties like
{{ array | where: '' }}would return an empty array. Now both empty strings and nil properties act as no-ops, simply returning the original array. This makes the library more forgiving when properties are missing or blank, reducing unexpected errors in your templates.