Skip to content

fix(web): use ES2022-safe array copies in client code - #1707

Merged
MODSetter merged 1 commit into
MODSetter:devfrom
Yigtwxx:fix/web-es2023-array-methods
Aug 24, 2026
Merged

fix(web): use ES2022-safe array copies in client code#1707
MODSetter merged 1 commit into
MODSetter:devfrom
Yigtwxx:fix/web-es2023-array-methods

Conversation

@Yigtwxx

@Yigtwxx Yigtwxx commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

The dashboard throws .filter(...).toSorted is not a function on Chrome 109 and any other browser below the ES2023 line. Array.prototype.toSorted and toReversed shipped in Chrome/Edge 110, Safari 16.4 and Firefox 115, and nothing in the build adds them. This replaces the five client-side uses with ES2022-safe equivalents.

Description

Five call sites in surfsense_web:

File Was Now
components/layout/ui/sidebar/NotificationsDropdown.tsx:127 .filter(...).toSorted(cmp) .filter(...).sort(cmp)
hooks/use-comments-sync.ts:122 (group.replies.get(raw.id) ?? []).toSorted(cmp) [...(group.replies.get(raw.id) ?? [])].sort(cmp)
components/free-chat/free-model-selector.tsx:51 models.toSorted(cmp) [...models].sort(cmp)
lib/chat/activity-journal.ts:92 [...activities].toSorted(cmp) [...activities].sort(cmp)
lib/chat/message-utils.ts:209 olderIdxs.toReversed() [...olderIdxs].reverse()

Where the receiver is already a fresh array — the filter result in NotificationsDropdown, the spread in activity-journal — sorting in place is safe and no extra copy is added. Everywhere else the receiver is state or a value held in a Map, so the copy is not optional; see below.

Symptom

Two auto-filed bug reports carry the same diagnostics block:

(intermediate value)(intermediate value)(intermediate value).filter(...).toSorted is not a function
Page:       /dashboard/<id>/new-chat
User agent: Chrome/109

#1645 and #1644 are the same defect, four minutes apart, from the same reporter. The stack points at NotificationsDropdown, where the call is inside a useMemo — so it throws during render and takes the notifications dropdown down with it, rather than failing quietly.

Root cause

surfsense_web/tsconfig.json sets "target": "ES2017". That downlevels syntax only; it never adds built-in methods, and there is no polyfill in the app. TypeScript does not warn either, because "lib" includes esnext, which declares toSorted/toReversed as available. There is no browserslist key in package.json and no .browserslistrc, so the support floor is implicit and nothing enforces it.

Two of these were also mutation bugs

  • hooks/use-comments-sync.ts:122 sorts group.replies.get(raw.id), an array stored in a Map that the same pass reads again. A naive .toSorted.sort swap there would reorder the stored array in place.
  • lib/chat/message-utils.ts:209 reverses mergeInto.get(i), also a stored array.

Both now copy before sorting, so the shared arrays are left alone. That is a small behavioural improvement, not just a compatibility fix.

What is deliberately not changed

app/(home)/changelog/page.tsx:36 also calls toSorted, and it is left alone: that page renders on the server, where Node 20+ has the method. Including it would have widened the diff without fixing a user-visible failure. The same reasoning applies to features/chat-messages/timeline/grouping.ts:168, which uses findLastIndex — also ES2023, but supported since Chrome 97, so it is not part of this failure.

One unrelated formatting fix, and why it is here

app/(home)/free/[model_slug]/page.tsx carries a format error that is already on dev:

Checked 1097 files in 2s. No fixes applied.
Found 1 error.

The biome-check-web hook in .pre-commit-config.yaml sets always_run: true with pass_filenames: false, so it checks the whole surfsense_web tree regardless of what a PR touched — the workflow's --from-ref/--to-ref narrowing does not reach it. That one error therefore fails Frontend Quality for every open PR, including backend-only ones. The fix is the unmodified output of biome check --write on that single file: a two-line reflow, no logic.

Motivation and Context

FIX #1645

#1644 is the same defect from the same reporter and is also resolved by this change. Since contributions merge into dev rather than the default branch, GitHub will not close either automatically.

Screenshots

Not applicable — no visual change. Sort order and rendering are identical.

API Changes

  • This PR includes API changes

Change Type

  • Bug fix
  • New feature
  • Performance improvement
  • Refactoring
  • Documentation
  • Dependency/Build system
  • Breaking change
  • Other (specify):

Testing Performed

  • Tested locally

  • Manual/QA verification

  • biome check --diagnostic-level=error . in surfsense_web, pinned 2.4.6: Checked 1097 files. No fixes applied. — zero errors, where dev reports one. Measured on an LF checkout, since a Windows working tree reports a format error on every file otherwise.

  • tsc --noEmit: 19 errors, the same 19 as on a clean dev checkout, none of them in the six files this PR touches. I did not try to fix them; they are unrelated.

  • Confirmed no toSorted/toReversed remains outside the server-rendered changelog page.

What does not change

  • Sort and reverse results. Every comparator is passed through unchanged, and Array.prototype.sort is stable in every engine this targets, so orderings are identical.
  • Component structure, hook dependency arrays, and rendering.
  • Type signatures: [...arr].sort(cmp) and arr.toSorted(cmp) both yield T[].
  • tsconfig.json. See below.

Remaining risk

Small, but worth naming: this fixes the five sites that exist today and adds no guard against the sixth. The obvious guard would be narrowing "lib" from esnext to es2022 in tsconfig.json, which turns any ES2023 built-in into a compile error. I did not include it, because it would also flag findLastIndex, which is fine on the browsers this bug is about, and the resulting rewrite would have nothing to do with the report. A browserslist entry documents intent but does not enforce it for built-ins. Happy to follow up with whichever you prefer.

Checklist

  • Follows project coding standards and conventions
  • Documentation updated as needed
  • Dependencies updated as needed
  • No lint/build errors or new warnings
  • All relevant tests are passing

High-level PR Summary

This PR fixes browser compatibility issues by replacing ES2023 methods (toSorted and toReversed) with ES2022-safe equivalents in client-side code. The changes prevent "is not a function" errors on Chrome 109 and other browsers that don't support ES2023 array methods. The fix converts five call sites to use array spread with sort() and reverse() instead, which also resolves two mutation bugs where shared arrays were being modified in place. An unrelated formatting fix in page.tsx is included to unblock CI checks.

⏱️ Estimated Review Time: 5-15 minutes

💡 Review Order Suggestion
Order File Path
1 surfsense_web/app/(home)/free/[model_slug]/page.tsx
2 surfsense_web/components/layout/ui/sidebar/NotificationsDropdown.tsx
3 surfsense_web/components/free-chat/free-model-selector.tsx
4 surfsense_web/lib/chat/activity-journal.ts
5 surfsense_web/hooks/use-comments-sync.ts
6 surfsense_web/lib/chat/message-utils.ts
⚠️ Inconsistent Changes Detected
File Path Warning
surfsense_web/app/(home)/free/[model_slug]/page.tsx This is a formatting-only change (text reflow) that is unrelated to the ES2022 compatibility fixes described in the PR. While the PR explains this was needed to fix CI, it's not part of the main bug fix.

Need help? Join our Discord

Array.prototype.toSorted and toReversed are ES2023 (Chrome/Edge 110,
Safari 16.4, Firefox 115). tsconfig targets ES2017, which downlevels syntax
but never adds built-ins, and lib includes esnext so TypeScript does not warn.
Five client-side call sites therefore throw on older browsers -- one of them
inside a useMemo, so it takes the notifications dropdown down during render.

Two of the five sorted an array held in a Map, so they copy before sorting
rather than switching to an in-place sort.

Also apply biome's formatting to app/(home)/free/[model_slug]/page.tsx. That
error is already on dev, and biome-check-web runs always_run with
pass_filenames: false, so it fails Frontend Quality for every open PR.
@vercel

vercel Bot commented Aug 21, 2026

Copy link
Copy Markdown

@Yigtwxx is attempting to deploy a commit to the Rohan Verma's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d36cb846-7b33-46f7-8d17-ba216eeeb87d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

@Yigtwxx

Yigtwxx commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Checks came back: Frontend Quality and Quality Gate are green here, which is worth noting because they are currently red on every other open PR — including backend-only ones that touch no TypeScript.

That is the always_run: true behaviour of the biome-check-web hook: it checks the whole surfsense_web tree regardless of what a PR changed, so the single format error sitting on dev in app/(home)/free/[model_slug]/page.tsx fails the gate for everyone. The two-line reflow in this PR is what clears it.

Vercel is the usual fork Authorization required to deploy, and recurseml/analysis errors on its own.

Journey is red here too, and for the same reason it is red everywhere: the stack never comes up. container surfsense-e2e-celery_worker-1 is unhealthy at bring-up, before any test runs, because the worker healthcheck shells out to celery -A app.celery_app inspect ping and -A imports the whole application (~40s in CI) against a timeout: 5s. That job has not passed in the last 60 runs, dev included. Fixed in #1708.

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.

2 participants