Skip to content

fix: Collection.sortBy() throws on frozen arrays in immutable cache mode - #2294

Merged
dfahlander merged 1 commit into
masterfrom
liz/fix-sortby-immutable-cache
Apr 8, 2026
Merged

fix: Collection.sortBy() throws on frozen arrays in immutable cache mode#2294
dfahlander merged 1 commit into
masterfrom
liz/fix-sortby-immutable-cache

Conversation

@liz709

@liz709 liz709 commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

Problem

When using cacheMode: 'immutable', toArray() returns frozen arrays. Calling .sort() directly on a frozen array throws:

TypeError: Cannot assign to read only property '0' of object '[object Array]'

Fix

Use .slice().sort() to sort on a copy rather than mutating the original frozen array.

Fixes

Fixes #2157

Summary by CodeRabbit

  • Bug Fixes
    • Fixed an issue where sorting operations would mutate the original array; the original data is now preserved.

When using immutable cache mode, toArray() returns frozen arrays.
Calling .sort() directly on a frozen array throws TypeError.
Fix by using .slice().sort() to sort on a copy instead.

Fixes #2157
@coderabbitai

coderabbitai Bot commented Apr 8, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 91ea7bf3-dfb3-470d-acb1-08fb4308f0ff

📥 Commits

Reviewing files that changed from the base of the PR and between 97f9942 and 07e45e5.

📒 Files selected for processing (1)
  • src/classes/collection/collection.ts

📝 Walkthrough

Walkthrough

Updated Collection.sortBy() to create a shallow copy of the array via .slice() before sorting, preventing mutation of the original array. This enables the method to work with immutable cache mode where returned arrays are read-only.

Changes

Cohort / File(s) Summary
Array Mutation Fix
src/classes/collection/collection.ts
Modified Collection.sortBy() to sort a shallow copy of the array via .slice() instead of mutating the original, enabling compatibility with immutable cache mode.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

A copy's made before we sort,
The frozen arrays come to port,
No mutations in immutable's way,
Our hopping code saves the day! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the fix: using slice() to avoid mutating frozen arrays returned by toArray() in immutable cache mode.
Linked Issues check ✅ Passed The code change directly addresses issue #2157 by fixing Collection.sortBy() to work with immutable cache mode through avoiding array mutations.
Out of Scope Changes check ✅ Passed The change is narrowly scoped to the specific problem: replacing a.sort() with a.slice().sort() in Collection.sortBy() method only.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch liz/fix-sortby-immutable-cache

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@dfahlander
dfahlander merged commit ac66e45 into master Apr 8, 2026
7 checks passed
@dfahlander
dfahlander deleted the liz/fix-sortby-immutable-cache branch April 8, 2026 08:45
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.

Is Collection.sortBy supported in immutable cache mode?

2 participants