Skip to content

Commit

Permalink
fix(eslint-plugin-router): ignore relative ordering of params and `…
Browse files Browse the repository at this point in the history
…validateSearch` (#2386)
  • Loading branch information
schiller-manuel authored Sep 20, 2024
1 parent 84dbdd8 commit 29eae76
Show file tree
Hide file tree
Showing 5 changed files with 122 additions and 33 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { RuleTester } from '@typescript-eslint/rule-tester'
import combinate from 'combinate'

import {
name,
rule,
Expand All @@ -23,6 +24,7 @@ const testedCheckedProperties = [
checkedProperties[0],
checkedProperties[1],
checkedProperties[2],
checkedProperties[3],
]
type TestedCheckedProperties = (typeof testedCheckedProperties)[number]
const orderIndependentProps = ['gcTime', '...foo'] as const
Expand All @@ -45,18 +47,49 @@ const validTestMatrix = combinate({
properties: generatePartialCombinations(testedCheckedProperties, 2),
})

export function generateInvalidPermutations<T>(
arr: ReadonlyArray<T>,
): Array<{ invalid: Array<T>; valid: Array<T> }> {
export function generateInvalidPermutations(
arr: ReadonlyArray<TestedCheckedProperties>,
): Array<{
invalid: Array<TestedCheckedProperties>
valid: Array<TestedCheckedProperties>
}> {
const combinations = generatePartialCombinations(arr, 2)
const allPermutations: Array<{ invalid: Array<T>; valid: Array<T> }> = []
const allPermutations: Array<{
invalid: Array<TestedCheckedProperties>
valid: Array<TestedCheckedProperties>
}> = []

for (const combination of combinations) {
const permutations = generatePermutations(combination)
// skip the first permutation as it matches the original combination
const invalidPermutations = permutations.slice(1)

if (
combination.includes('params') &&
combination.includes('validateSearch')
) {
if (
combination.indexOf('params') < combination.indexOf('validateSearch')
) {
// since we ignore the relative order of 'params' and 'validateSearch', we skip this combination (but keep the other one where `validateSearch` is before `params`)
continue
}
}

allPermutations.push(
...invalidPermutations.map((p) => ({ invalid: p, valid: combination })),
...invalidPermutations.map((p) => {
// ignore the relative order of 'params' and 'validateSearch'
const correctedValid = [...combination].sort((a, b) => {
if (
(a === 'params' && b === 'validateSearch') ||
(a === 'validateSearch' && b === 'params')
) {
return p.indexOf(a) - p.indexOf(b)
}
return checkedProperties.indexOf(a) - checkedProperties.indexOf(b)
})
return { invalid: p, valid: correctedValid }
}),
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,43 +6,64 @@ describe('create-route-property-order utils', () => {
const testCases = [
{
data: [{ key: 'a' }, { key: 'c' }, { key: 'b' }],
orderArray: ['a', 'b', 'c'],
orderArray: [
[['a'], ['b']],
[['b'], ['c']],
],
key: 'key',
expected: [{ key: 'a' }, { key: 'b' }, { key: 'c' }],
},
{
data: [{ key: 'b' }, { key: 'a' }, { key: 'c' }],
orderArray: ['a', 'b', 'c'],
orderArray: [
[['a'], ['b']],
[['b'], ['c']],
],
key: 'key',
expected: [{ key: 'a' }, { key: 'b' }, { key: 'c' }],
},
{
data: [{ key: 'a' }, { key: 'b' }, { key: 'c' }],
orderArray: ['a', 'b', 'c'],
orderArray: [
[['a'], ['b']],
[['b'], ['c']],
],
key: 'key',
expected: null,
},
{
data: [{ key: 'a' }, { key: 'b' }, { key: 'c' }, { key: 'd' }],
orderArray: ['a', 'b', 'c'],
orderArray: [
[['a'], ['b']],
[['b'], ['c']],
],
key: 'key',
expected: null,
},
{
data: [{ key: 'a' }, { key: 'b' }, { key: 'd' }, { key: 'c' }],
orderArray: ['a', 'b', 'c'],
orderArray: [
[['a'], ['b']],
[['b'], ['c']],
],
key: 'key',
expected: null,
},
{
data: [{ key: 'd' }, { key: 'a' }, { key: 'b' }, { key: 'c' }],
orderArray: ['a', 'b', 'c'],
orderArray: [
[['a'], ['b']],
[['b'], ['c']],
],
key: 'key',
expected: null,
},
{
data: [{ key: 'd' }, { key: 'b' }, { key: 'a' }, { key: 'c' }],
orderArray: ['a', 'b', 'c'],
orderArray: [
[['a'], ['b']],
[['b'], ['c']],
],
key: 'key',
expected: [{ key: 'd' }, { key: 'a' }, { key: 'b' }, { key: 'c' }],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,10 @@ export const checkedProperties = [
'loaderDeps',
'loader',
] as const

export const sortRules = [
[['params', 'validateSearch'], ['context']],
[['context'], ['beforeLoad']],
[['beforeLoad'], ['loaderDeps']],
[['loaderDeps'], ['loader']],
] as const
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import { getDocsUrl } from '../../utils/get-docs-url'
import { detectTanstackRouterImports } from '../../utils/detect-router-imports'
import { sortDataByOrder } from './create-route-property-order.utils'
import {
checkedProperties,
createRouteFunctions,
createRouteFunctionsIndirect,
sortRules,
} from './constants'
import type { CreateRouteFunction } from './constants'
import type { ExtraRuleDocs } from '../../types'
Expand Down Expand Up @@ -63,8 +63,11 @@ export const rule = createRule({
}

const allProperties = argument.properties
// no need to sort if there is at max 1 property
if (allProperties.length < 2) {
return
}

// TODO we need to support spread elements, they would be discarded here
const properties = allProperties.flatMap((p) => {
if (
p.type === AST_NODE_TYPES.Property &&
Expand All @@ -81,11 +84,7 @@ export const rule = createRule({
return []
})

const sortedProperties = sortDataByOrder(
properties,
checkedProperties,
'name',
)
const sortedProperties = sortDataByOrder(properties, sortRules, 'name')
if (sortedProperties === null) {
return
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,27 +1,56 @@
export function sortDataByOrder<T, TKey extends keyof T>(
data: Array<T> | ReadonlyArray<T>,
orderArray: Array<T[TKey]> | ReadonlyArray<T[TKey]>,
orderRules: ReadonlyArray<
Readonly<[ReadonlyArray<T[TKey]>, ReadonlyArray<T[TKey]>]>
>,
key: TKey,
): Array<T> | null {
const orderMap = new Map(orderArray.map((item, index) => [item, index]))

// Separate items that are in orderArray from those that are not
const inOrderArray = data
.filter((item) => orderMap.has(item[key]))
.sort((a, b) => {
const indexA = orderMap.get(a[key])!
const indexB = orderMap.get(b[key])!
const getSubsetIndex = (
item: T[TKey],
subsets: ReadonlyArray<ReadonlyArray<T[TKey]> | Array<T[TKey]>>,
): number | null => {
for (let i = 0; i < subsets.length; i++) {
if (subsets[i]?.includes(item)) {
return i
}
}
return null
}

return indexA - indexB
})
const orderSets = orderRules.reduce(
(sets, [A, B]) => [...sets, A, B],
[] as Array<ReadonlyArray<T[TKey]> | Array<T[TKey]>>,
)

const inOrderIterator = inOrderArray.values()
const inOrderArray = data.filter(
(item) => getSubsetIndex(item[key], orderSets) !== null,
)

// `as boolean` is needed to avoid TS incorrectly inferring that wasResorted is always `true`
let wasResorted = false as boolean

// Sort by the relative order defined by the rules
const sortedArray = inOrderArray.sort((a, b) => {
const aKey = a[key],
bKey = b[key]
const aSubsetIndex = getSubsetIndex(aKey, orderSets)
const bSubsetIndex = getSubsetIndex(bKey, orderSets)

// If both items belong to different subsets, sort by their subset order
if (
aSubsetIndex !== null &&
bSubsetIndex !== null &&
aSubsetIndex !== bSubsetIndex
) {
return aSubsetIndex - bSubsetIndex
}

// If both items belong to the same subset or neither is in the subset, keep their relative order
return 0
})

const inOrderIterator = sortedArray.values()
const result = data.map((item) => {
if (orderMap.has(item[key])) {
if (getSubsetIndex(item[key], orderSets) !== null) {
const sortedItem = inOrderIterator.next().value!
if (sortedItem[key] !== item[key]) {
wasResorted = true
Expand Down

0 comments on commit 29eae76

Please sign in to comment.