Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 112 additions & 1 deletion packages/query-devtools/src/__tests__/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@ import {
getMutationStatusColor,
getQueryStatusColorByLabel,
getSidedProp,
mutationSortFns,
setupStyleSheet,
sortFns,
updateNestedDataByPath,
} from '../utils'
import type { MutationStatus, Query } from '@tanstack/query-core'
import type { Mutation, MutationStatus, Query } from '@tanstack/query-core'

describe('Utils tests', () => {
describe('updatedNestedDataByPath', () => {
Expand Down Expand Up @@ -1067,4 +1068,114 @@ describe('Utils tests', () => {
})
})
})

describe('mutationSortFns', () => {
let queryClient: QueryClient

const defaultMutationState: Mutation['state'] = {
context: undefined,
data: undefined,
error: null,
failureCount: 0,
failureReason: null,
isPaused: false,
status: 'idle',
variables: undefined,
submittedAt: 0,
}

function buildMutation(overrides: Partial<Mutation['state']>): Mutation {
return queryClient
.getMutationCache()
.build(queryClient, {}, { ...defaultMutationState, ...overrides })
}

beforeEach(() => {
queryClient = new QueryClient()
})

afterEach(() => {
queryClient.clear()
})

describe("'last updated'", () => {
const mutationDateSort = mutationSortFns['last updated']!

it('should place the more recently submitted mutation first', () => {
const older = buildMutation({ submittedAt: 100 })
const newer = buildMutation({ submittedAt: 200 })

expect(mutationDateSort(older, newer)).toBe(1)
expect(mutationDateSort(newer, older)).toBe(-1)
Comment on lines +1108 to +1109
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.

⚠️ Potential issue | 🟑 Minor | ⚑ Quick win

🧩 Analysis chain

🌐 Web query:

For JavaScript Array.prototype.sort, must a comparator return exactly -1/0/1, or is any negative/zero/positive number valid?

πŸ’‘ Result:

Any negative/zero/positive number is valid for the comparator in JavaScript Array.prototype.sort. The specification and documentation define the behavior based on the sign of the return value: less than 0 means a before b, 0 means equal (original order preserved where stable), greater than 0 means a after b [1][2][3]. Examples like numeric sorting with (a - b), which can return values like -60, are standard and recommended [4][1]. MDN explicitly uses < 0, === 0, > 0 in its table, not requiring exactly -1/0/1 [1]. While examples often show -1/0/1 for clarity, any value with the correct sign works, as sorting algorithms (like quicksort, timsort) only check the sign [1][5]. The ECMAScript spec's SortCompare abstract operation treats the result based on whether it's less than, equal to, or greater than zero after coercion [6][7]. Returning exactly -1/0/1 is a best practice for readability and to avoid large number precision issues with floats, but not required [1].

Citations:


🏁 Script executed:

# Check the test file content at the specified lines
sed -n '1108,1109p; 1128,1129p; 1144,1145p; 1160,1161p; 1176,1177p' packages/query-devtools/src/__tests__/utils.test.ts

Repository: TanStack/query

Length of output: 647


🏁 Script executed:

# Find where mutationDateSort and mutationStatusSort are defined
rg -l "mutationDateSort|mutationStatusSort" packages/query-devtools/src --type ts --type tsx

Repository: TanStack/query

Length of output: 86


🏁 Script executed:

# Find where mutationDateSort and mutationStatusSort are defined
rg "mutationDateSort|mutationStatusSort" packages/query-devtools/src -l

Repository: TanStack/query

Length of output: 148


🏁 Script executed:

# Find and examine the implementations
rg "mutationDateSort|mutationStatusSort" packages/query-devtools/src/utils.tsx -A 5

Repository: TanStack/query

Length of output: 599


Assert comparator sign, not exact -1/1.

While these comparators currently return exactly -1 and 1, the test assertions encode this implementation detail unnecessarily. JavaScript sort comparators only need to return negative/zero/positive values. Using toBeGreaterThan(0) and toBeLessThan(0) makes tests more resilient to refactoringβ€”the assertions will pass if implementations return other signed values (e.g., 2 / -2), which is semantically correct for sorting.

Suggested update
-        expect(mutationDateSort(older, newer)).toBe(1)
-        expect(mutationDateSort(newer, older)).toBe(-1)
+        expect(mutationDateSort(older, newer)).toBeGreaterThan(0)
+        expect(mutationDateSort(newer, older)).toBeLessThan(0)

-        expect(mutationStatusSort(paused, pending)).toBe(-1)
-        expect(mutationStatusSort(pending, paused)).toBe(1)
+        expect(mutationStatusSort(paused, pending)).toBeLessThan(0)
+        expect(mutationStatusSort(pending, paused)).toBeGreaterThan(0)

-        expect(mutationStatusSort(pending, success)).toBe(-1)
-        expect(mutationStatusSort(success, pending)).toBe(1)
+        expect(mutationStatusSort(pending, success)).toBeLessThan(0)
+        expect(mutationStatusSort(success, pending)).toBeGreaterThan(0)

-        expect(mutationStatusSort(error, success)).toBe(-1)
-        expect(mutationStatusSort(success, error)).toBe(1)
+        expect(mutationStatusSort(error, success)).toBeLessThan(0)
+        expect(mutationStatusSort(success, error)).toBeGreaterThan(0)

-        expect(mutationStatusSort(older, newer)).toBe(1)
-        expect(mutationStatusSort(newer, older)).toBe(-1)
+        expect(mutationStatusSort(older, newer)).toBeGreaterThan(0)
+        expect(mutationStatusSort(newer, older)).toBeLessThan(0)
πŸ“ Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
expect(mutationDateSort(older, newer)).toBe(1)
expect(mutationDateSort(newer, older)).toBe(-1)
expect(mutationDateSort(older, newer)).toBeGreaterThan(0)
expect(mutationDateSort(newer, older)).toBeLessThan(0)
πŸ€– Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/query-devtools/src/__tests__/utils.test.ts` around lines 1108 -
1109, The tests assert exact comparator return values; update the two assertions
that call mutationDateSort(older, newer) and mutationDateSort(newer, older) to
assert sign rather than exact numbers β€” replace the toBe(1) check with
toBeGreaterThan(0) and the toBe(-1) check with toBeLessThan(0) so the tests only
require positive/negative results from mutationDateSort when comparing older and
newer.

})
})

describe("'status'", () => {
const mutationStatusSort = mutationSortFns['status']!

it('should place a paused mutation first', () => {
const paused = buildMutation({
isPaused: true,
status: 'pending',
submittedAt: 100,
})
const pending = buildMutation({
isPaused: false,
status: 'pending',
submittedAt: 100,
})

expect(mutationStatusSort(paused, pending)).toBe(-1)
expect(mutationStatusSort(pending, paused)).toBe(1)
})

it('should place a pending mutation before a successful one', () => {
const pending = buildMutation({
isPaused: false,
status: 'pending',
submittedAt: 100,
})
const success = buildMutation({
isPaused: false,
status: 'success',
submittedAt: 100,
})

expect(mutationStatusSort(pending, success)).toBe(-1)
expect(mutationStatusSort(success, pending)).toBe(1)
})

it('should place an errored mutation before a successful one', () => {
const error = buildMutation({
isPaused: false,
status: 'error',
submittedAt: 100,
})
const success = buildMutation({
isPaused: false,
status: 'success',
submittedAt: 100,
})

expect(mutationStatusSort(error, success)).toBe(-1)
expect(mutationStatusSort(success, error)).toBe(1)
})

it('should fall back to "last updated" sort within the same status rank', () => {
const older = buildMutation({
isPaused: false,
status: 'success',
submittedAt: 100,
})
const newer = buildMutation({
isPaused: false,
status: 'success',
submittedAt: 200,
})

expect(mutationStatusSort(older, newer)).toBe(1)
expect(mutationStatusSort(newer, older)).toBe(-1)
})
})
})
})
Loading