Skip to content

fix: allow replace() and replaceAll() helpers to handle array inputs#1237

Closed
bhuxhorn wants to merge 4 commits into
adobe:mainfrom
bhuxhorn:bugfix/1236-fix-indexer-replace
Closed

fix: allow replace() and replaceAll() helpers to handle array inputs#1237
bhuxhorn wants to merge 4 commits into
adobe:mainfrom
bhuxhorn:bugfix/1236-fix-indexer-replace

Conversation

@bhuxhorn

Copy link
Copy Markdown
Contributor

Description

What the changes intend

This PR fixes a TypeError: s.replace is not a function crash that occurs when the replace() or replaceAll() value expression helpers in helix-query.yaml receive an array as their first argument (such as the output from the attribute() helper).

While other indexer helpers like match, words, and characters inherently handle array inputs, replace and replaceAll previously expected a strict string. This change aligns their behavior with the rest of the helpers and ensures the documented example for combining replace() and attribute() works as expected.

How they change the existing code

  • Modified the replace and replaceAll helper functions to detect if the incoming value is an array.
  • If an array is passed, the helpers now map over the elements and apply the string replacement to each item individually.
  • Added regression tests to validate both helpers when processing string and array inputs.

If (and what) they break

  • Breaking changes: None. This is a backward-compatible bug fix that prevents a runtime crash on configurations that are currently broken.

Related Issue

Fixes #1236

@tripodsan

Copy link
Copy Markdown
Contributor

thanks @bhuxhorn , please sign the Adobe CLA.

@tripodsan

Copy link
Copy Markdown
Contributor

thanks. I want to wait for @dominique-pfister 's review.

Comment on lines +129 to +133
if (Array.isArray(s)) {
return s.map((item) => (
item != null ? String(item).replace(searchValue, replaceValue) : item
));
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me as well, just one thing: should we rather filter items that are null (or undefined) instead of returning them in the array?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done - nulls are now filtered out before mapping; updated the test expectations accordingly.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, and using the loose un-equality, this will filter out both null and undefined array values, which is fine for me.

@tripodsan

Copy link
Copy Markdown
Contributor

fixed with. #1240

@tripodsan tripodsan closed this Jul 1, 2026
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

Successfully merging this pull request may close these issues.

replace() and replaceAll() in the indexer crash when composed with attribute()

4 participants