Fix flatten hit modify original array

[wanglam]

Description

This PR addresses an issue with the flattenHit function modifying the original hit object array. Currently, when the flattenHit function encounters an array value within the hit object, it flattens the array in-place, directly modifying the original hit object.

The proposed changes introduce a check to determine if the value is an array. If it is, a new array is constructed with the flattened values, preserving the original hit object array. This ensures that the flattenHit function no longer mutates the original hit object, preventing potential side effects or unintended modifications.

Issues Resolved

#9318

Screenshot

Testing the changes

Following the reproduce steps in #9318

Changelog

• fix: flatten hit modify original array

Check List

• All tests pass
  • yarn test:jest
  • yarn test:jest_integration
• New functionality includes testing.
• New functionality has been documented.
• Update CHANGELOG.md
• Commits are signed per the DCO using --signoff

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.71%. Comparing base (21f1f73) to head (8edf8d5).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9347      +/-   ##
==========================================
+ Coverage   61.68%   61.71%   +0.02%     
==========================================
  Files        3816     3816              
  Lines       91829    91829              
  Branches    14543    14544       +1     
==========================================
+ Hits        56649    56671      +22     
+ Misses      31521    31504      -17     
+ Partials     3659     3654       -5     

Flag	Coverage Δ
Linux_1	28.99% <0.00%> (-0.01%)	⬇️
Linux_2	56.46% <0.00%> (ø)
Linux_3	?
Linux_4	28.90% <0.00%> (ø)
Windows_1	29.00% <0.00%> (?)
Windows_2	56.41% <0.00%> (ø)
Windows_3	39.19% <100.00%> (?)
Windows_4	28.90% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Maosaic]

Nit: imo it's more readable by using the spread operator for cloning, or maybe add some context as comment.

[d-buckner]

Just for awareness (feel free to ignore if this is known): The Array.prototype.concat API creates a shallow copy. If there are modifications that are made in the array item objects themselves, they will impact the original hits object given they share references.

[Maosaic]

If we need deep copy here, we may want to use structuredClone

[wanglam]

Thanks for catching that. I will refactor with the spread operator instead of Array.prototype.concat. Do you think we should use a deep copy here? This PR is for fixing the issue of the original hit being modified inside this flattenHit function. The flat[key].push in line 67 will push elements to the original array. If we want to do a deep copy, we may need to change lines 67 and 69 too.

[d-buckner]

@wanglam If the shallow copy resolves the issue then I think it makes sense to avoid deep cloning as it's relatively expensive. If in the future we see cases where the items are modified as well we can always move to using deep cloning then. Thanks for taking the time to fix.