Skip to content

fix: resolve sort comparator and code quality issues (@ennajari)#7805

Open
ennajari wants to merge 4 commits intomonkeytypegame:masterfrom
ennajari:fix/sort-comparator-and-code-quality
Open

fix: resolve sort comparator and code quality issues (@ennajari)#7805
ennajari wants to merge 4 commits intomonkeytypegame:masterfrom
ennajari:fix/sort-comparator-and-code-quality

Conversation

@ennajari
Copy link
Copy Markdown

@ennajari ennajari commented Apr 10, 2026

Summary

  • Use locale-independent sort comparator (a, b) => (a < b ? -1 : a > b ? 1 : 0) in .sort() calls for consistent ordering regardless of runtime locale (setters.ts, Sidebar.tsx, themes.ts)
  • Add initial value 0 to reduce() in stdDev to safely handle edge cases
  • Use explicit !== undefined check instead of truthy check for cached Promise in memoize utility

Test plan

  • Funbox selection still sorts correctly
  • Leaderboard sidebar mode/language selection works
  • Theme list remains consistently ordered
  • stdDev([]) returns 0 instead of NaN
  • Memoized async functions cache correctly

- Use localeCompare in sort calls to ensure consistent ordering across
  locales (setters.ts, Sidebar.tsx, connections.ts, themes.ts)
- Add initial value 0 to reduce() in stdDev to handle empty arrays safely
- Use explicit undefined check for cached Promise in memoize utility
- Add NOSONAR annotations for intentional patterns (thenable object,
  CharacterCounter side-effect instantiation)
- Add .scannerwork/, .claude/, sonar-project.properties to .gitignore
@monkeytypegeorge monkeytypegeorge added backend Server stuff frontend User interface or web stuff packages Changes in local packages labels Apr 10, 2026
@ennajari ennajari changed the title fix: resolve sort comparator and code quality issues fix: resolve sort comparator and code quality issues (@ennajari) Apr 10, 2026
function getKey(initiatorUid: string, receiverUid: string): string {
const ids = [initiatorUid, receiverUid];
ids.sort();
ids.sort((a, b) => a.localeCompare(b));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not needed for backend

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Reverted — is back to plain .

@fehmer
Copy link
Copy Markdown
Member

fehmer commented Apr 10, 2026

We don't use sonar, please remove sonar instructions and .ignore

@monkeytypegeorge monkeytypegeorge removed the backend Server stuff label Apr 10, 2026
} else {
newConfig.push(funbox);
newConfig.sort();
newConfig.sort((a, b) => a.localeCompare(b));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we want consistent sorting here and not depend on the users locale.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Changed to a locale-independent comparator: (a, b) => (a < b ? -1 : a > b ? 1 : 0). Same for Sidebar.tsx and themes.ts.

@fehmer
Copy link
Copy Markdown
Member

fehmer commented Apr 10, 2026

Hi @ennajari , thanks for the changes. The pr summary doesn't match the or content anymore

@ennajari ennajari requested a review from fehmer April 10, 2026 17:15
@github-actions github-actions bot added the waiting for review Pull requests that require a review before continuing label Apr 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend User interface or web stuff packages Changes in local packages waiting for review Pull requests that require a review before continuing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants