Skip to content

Commit 398ac13

Browse files
hugop95hugop95
authoredOct 15, 2024··
feat: Improve side-effect handling behavior in sort-imports
·
v4.11.0v3.9.0
1 parent a7d3f8c commit 398ac13

6 files changed

+718
-345
lines changed
 

‎rules/sort-classes.ts

+12-42
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
sortNodesByDependencies,
2626
} from '../utils/sort-nodes-by-dependencies'
2727
import { hasPartitionComment } from '../utils/is-partition-comment'
28+
import { sortNodesByGroups } from '../utils/sort-nodes-by-groups'
2829
import { getCommentsBefore } from '../utils/get-comments-before'
2930
import { createEslintRule } from '../utils/create-eslint-rule'
3031
import { getGroupNumber } from '../utils/get-group-number'
@@ -33,7 +34,6 @@ import { toSingleLine } from '../utils/to-single-line'
3334
import { rangeToDiff } from '../utils/range-to-diff'
3435
import { getSettings } from '../utils/get-settings'
3536
import { useGroups } from '../utils/use-groups'
36-
import { sortNodes } from '../utils/sort-nodes'
3737
import { makeFixes } from '../utils/make-fixes'
3838
import { complete } from '../utils/complete'
3939
import { pairwise } from '../utils/pairwise'
@@ -649,47 +649,17 @@ export default createEslintRule<SortClassesOptions, MESSAGE_ID>({
649649
[[]],
650650
)
651651

652-
let sortedNodes: SortingNodeWithDependencies[] = []
653-
for (let nodes of formattedNodes) {
654-
let nodesByNonIgnoredGroupNumber: {
655-
[key: number]: SortingNodeWithDependencies[]
656-
} = {}
657-
let ignoredNodeIndices: number[] = []
658-
for (let [index, sortingNode] of nodes.entries()) {
659-
let groupNum = getGroupNumber(options.groups, sortingNode)
660-
if (groupNum === options.groups.length) {
661-
ignoredNodeIndices.push(index)
662-
continue
663-
}
664-
nodesByNonIgnoredGroupNumber[groupNum] =
665-
nodesByNonIgnoredGroupNumber[groupNum] ?? []
666-
nodesByNonIgnoredGroupNumber[groupNum].push(sortingNode)
667-
}
668-
669-
for (let groupNumber of Object.keys(
670-
nodesByNonIgnoredGroupNumber,
671-
).sort((a, b) => Number(a) - Number(b))) {
672-
let compareOptions = getCompareOptions(options, Number(groupNumber))
673-
if (!compareOptions) {
674-
// Do not sort this group
675-
sortedNodes.push(
676-
...nodesByNonIgnoredGroupNumber[Number(groupNumber)],
677-
)
678-
} else {
679-
sortedNodes.push(
680-
...sortNodes(
681-
nodesByNonIgnoredGroupNumber[Number(groupNumber)],
682-
compareOptions,
683-
),
684-
)
685-
}
686-
}
687-
688-
// Add ignored nodes at the same position as they were before linting
689-
for (let ignoredIndex of ignoredNodeIndices) {
690-
sortedNodes.splice(ignoredIndex, 0, nodes[ignoredIndex])
691-
}
692-
}
652+
let sortedNodes = formattedNodes
653+
.map(nodes =>
654+
sortNodesByGroups(nodes, options, {
655+
getGroupCompareOptions: groupNumber =>
656+
getCompareOptions(options, groupNumber),
657+
isNodeIgnored: sortingNode =>
658+
getGroupNumber(options.groups, sortingNode) ===
659+
options.groups.length,
660+
}),
661+
)
662+
.flat()
693663

694664
sortedNodes = sortNodesByDependencies(sortedNodes)
695665
let nodes = formattedNodes.flat()

‎rules/sort-imports.ts

+231-279
Large diffs are not rendered by default.

‎test/clean-groups-option.test.ts

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { describe, expect, it } from 'vitest'
2+
3+
import { getOptionsWithCleanGroups } from '../utils/get-options-with-clean-groups'
4+
5+
describe('get-options-with-cleaned-groups', () => {
6+
it('get options with cleaned groups', () => {
7+
expect(
8+
getOptionsWithCleanGroups({
9+
groups: [
10+
'predefinedGroup',
11+
[],
12+
['customGroup', 'group1'],
13+
['singleGroup'],
14+
'group2',
15+
],
16+
}),
17+
).toStrictEqual({
18+
groups: [
19+
'predefinedGroup',
20+
['customGroup', 'group1'],
21+
'singleGroup',
22+
'group2',
23+
],
24+
})
25+
})
26+
})

‎test/sort-imports.test.ts

+395-18
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
1+
import type { RuleContext } from '@typescript-eslint/utils/ts-eslint'
2+
13
import { RuleTester } from '@typescript-eslint/rule-tester'
2-
import { afterAll, describe, it } from 'vitest'
4+
import { afterAll, describe, expect, it } from 'vitest'
35
import { dedent } from 'ts-dedent'
46

7+
import type { MESSAGE_ID, Options } from '../rules/sort-imports'
8+
59
import rule from '../rules/sort-imports'
610

711
let ruleName = 'sort-imports'
@@ -155,9 +159,9 @@ describe(ruleName, () => {
155159
156160
import a from '.'
157161
import h from '../../h'
162+
import './style.css'
158163
import { j } from '../j'
159164
import { K, L, M } from '../k'
160-
import './style.css'
161165
`,
162166
options: [
163167
{
@@ -248,13 +252,6 @@ describe(ruleName, () => {
248252
right: './style.css',
249253
},
250254
},
251-
{
252-
messageId: 'unexpectedImportsOrder',
253-
data: {
254-
left: './style.css',
255-
right: '../j',
256-
},
257-
},
258255
],
259256
},
260257
],
@@ -1554,6 +1551,331 @@ describe(ruleName, () => {
15541551
},
15551552
)
15561553

1554+
describe(`${ruleName}(${type}): disabling side-effect sorting`, () => {
1555+
ruleTester.run(
1556+
`${ruleName}(${type}): allows 'side-effect' and 'side-effect-style' groups to stay in place`,
1557+
rule,
1558+
{
1559+
valid: [],
1560+
invalid: [
1561+
{
1562+
code: dedent`
1563+
import "./z-side-effect.scss";
1564+
import b from "./b";
1565+
import './b-side-effect'
1566+
import "./g-side-effect.css";
1567+
import './a-side-effect'
1568+
import a from "./a";
1569+
`,
1570+
output: dedent`
1571+
import "./z-side-effect.scss";
1572+
import a from "./a";
1573+
import './b-side-effect'
1574+
import "./g-side-effect.css";
1575+
import './a-side-effect'
1576+
import b from "./b";
1577+
`,
1578+
options: [
1579+
{
1580+
...options,
1581+
newlinesBetween: 'always',
1582+
groups: ['unknown'],
1583+
},
1584+
],
1585+
errors: [
1586+
{
1587+
messageId: 'unexpectedImportsOrder',
1588+
data: {
1589+
left: './b',
1590+
right: './b-side-effect',
1591+
},
1592+
},
1593+
{
1594+
messageId: 'unexpectedImportsOrder',
1595+
data: {
1596+
left: './a-side-effect',
1597+
right: './a',
1598+
},
1599+
},
1600+
],
1601+
},
1602+
],
1603+
},
1604+
)
1605+
1606+
ruleTester.run(
1607+
`${ruleName}(${type}): allows 'side-effect' to be grouped together but not sorted`,
1608+
rule,
1609+
{
1610+
valid: [],
1611+
invalid: [
1612+
{
1613+
code: dedent`
1614+
import "./z-side-effect.scss";
1615+
import b from "./b";
1616+
import './b-side-effect'
1617+
import "./g-side-effect.css";
1618+
import './a-side-effect'
1619+
import a from "./a";
1620+
`,
1621+
output: dedent`
1622+
import "./z-side-effect.scss";
1623+
import './b-side-effect'
1624+
import "./g-side-effect.css";
1625+
import './a-side-effect'
1626+
1627+
import a from "./a";
1628+
import b from "./b";
1629+
`,
1630+
options: [
1631+
{
1632+
...options,
1633+
newlinesBetween: 'always',
1634+
groups: ['side-effect', 'unknown'],
1635+
},
1636+
],
1637+
errors: [
1638+
{
1639+
messageId: 'missedSpacingBetweenImports',
1640+
data: {
1641+
left: './z-side-effect.scss',
1642+
right: './b',
1643+
},
1644+
},
1645+
{
1646+
messageId: 'unexpectedImportsGroupOrder',
1647+
data: {
1648+
left: './b',
1649+
leftGroup: 'unknown',
1650+
right: './b-side-effect',
1651+
rightGroup: 'side-effect',
1652+
},
1653+
},
1654+
{
1655+
messageId: 'missedSpacingBetweenImports',
1656+
data: {
1657+
left: './a-side-effect',
1658+
right: './a',
1659+
},
1660+
},
1661+
],
1662+
},
1663+
],
1664+
},
1665+
)
1666+
1667+
ruleTester.run(
1668+
`${ruleName}(${type}): allows 'side-effect' and 'side-effect-style' to be grouped together
1669+
in the same group but not sorted`,
1670+
rule,
1671+
{
1672+
valid: [],
1673+
invalid: [
1674+
{
1675+
code: dedent`
1676+
import "./z-side-effect.scss";
1677+
import b from "./b";
1678+
import './b-side-effect'
1679+
import "./g-side-effect.css";
1680+
import './a-side-effect'
1681+
import a from "./a";
1682+
`,
1683+
output: dedent`
1684+
import "./z-side-effect.scss";
1685+
import './b-side-effect'
1686+
import "./g-side-effect.css";
1687+
import './a-side-effect'
1688+
1689+
import a from "./a";
1690+
import b from "./b";
1691+
`,
1692+
options: [
1693+
{
1694+
...options,
1695+
newlinesBetween: 'always',
1696+
groups: [['side-effect', 'side-effect-style'], 'unknown'],
1697+
},
1698+
],
1699+
errors: [
1700+
{
1701+
messageId: 'missedSpacingBetweenImports',
1702+
data: {
1703+
left: './z-side-effect.scss',
1704+
right: './b',
1705+
},
1706+
},
1707+
{
1708+
messageId: 'unexpectedImportsGroupOrder',
1709+
data: {
1710+
left: './b',
1711+
leftGroup: 'unknown',
1712+
right: './b-side-effect',
1713+
rightGroup: 'side-effect',
1714+
},
1715+
},
1716+
{
1717+
messageId: 'missedSpacingBetweenImports',
1718+
data: {
1719+
left: './a-side-effect',
1720+
right: './a',
1721+
},
1722+
},
1723+
],
1724+
},
1725+
],
1726+
},
1727+
)
1728+
1729+
ruleTester.run(
1730+
`${ruleName}(${type}): allows 'side-effect' and 'side-effect-style' to be grouped together but not sorted`,
1731+
rule,
1732+
{
1733+
valid: [],
1734+
invalid: [
1735+
{
1736+
code: dedent`
1737+
import "./z-side-effect.scss";
1738+
import b from "./b";
1739+
import './b-side-effect'
1740+
import "./g-side-effect.css";
1741+
import './a-side-effect'
1742+
import a from "./a";
1743+
`,
1744+
output: dedent`
1745+
import './b-side-effect'
1746+
import './a-side-effect'
1747+
1748+
import "./z-side-effect.scss";
1749+
import "./g-side-effect.css";
1750+
1751+
import a from "./a";
1752+
import b from "./b";
1753+
`,
1754+
options: [
1755+
{
1756+
...options,
1757+
newlinesBetween: 'always',
1758+
groups: ['side-effect', 'side-effect-style', 'unknown'],
1759+
},
1760+
],
1761+
errors: [
1762+
{
1763+
messageId: 'missedSpacingBetweenImports',
1764+
data: {
1765+
left: './z-side-effect.scss',
1766+
right: './b',
1767+
},
1768+
},
1769+
{
1770+
messageId: 'unexpectedImportsGroupOrder',
1771+
data: {
1772+
left: './b',
1773+
leftGroup: 'unknown',
1774+
right: './b-side-effect',
1775+
rightGroup: 'side-effect',
1776+
},
1777+
},
1778+
{
1779+
messageId: 'missedSpacingBetweenImports',
1780+
data: {
1781+
left: './b-side-effect',
1782+
right: './g-side-effect.css',
1783+
},
1784+
},
1785+
{
1786+
messageId: 'unexpectedImportsGroupOrder',
1787+
data: {
1788+
left: './g-side-effect.css',
1789+
leftGroup: 'side-effect-style',
1790+
right: './a-side-effect',
1791+
rightGroup: 'side-effect',
1792+
},
1793+
},
1794+
{
1795+
messageId: 'missedSpacingBetweenImports',
1796+
data: {
1797+
left: './a-side-effect',
1798+
right: './a',
1799+
},
1800+
},
1801+
],
1802+
},
1803+
],
1804+
},
1805+
)
1806+
1807+
ruleTester.run(
1808+
`${ruleName}(${type}): allows 'side-effect-style' to be grouped together but not sorted`,
1809+
rule,
1810+
{
1811+
valid: [],
1812+
invalid: [
1813+
{
1814+
code: dedent`
1815+
import "./z-side-effect";
1816+
import b from "./b";
1817+
import './b-side-effect.scss'
1818+
import "./g-side-effect";
1819+
import './a-side-effect.css'
1820+
import a from "./a";
1821+
`,
1822+
output: dedent`
1823+
import "./z-side-effect";
1824+
1825+
import './b-side-effect.scss'
1826+
import './a-side-effect.css'
1827+
1828+
import "./g-side-effect";
1829+
import a from "./a";
1830+
import b from "./b";
1831+
`,
1832+
options: [
1833+
{
1834+
...options,
1835+
newlinesBetween: 'always',
1836+
groups: ['side-effect-style', 'unknown'],
1837+
},
1838+
],
1839+
errors: [
1840+
{
1841+
messageId: 'unexpectedImportsGroupOrder',
1842+
data: {
1843+
left: './b',
1844+
leftGroup: 'unknown',
1845+
right: './b-side-effect.scss',
1846+
rightGroup: 'side-effect-style',
1847+
},
1848+
},
1849+
{
1850+
messageId: 'missedSpacingBetweenImports',
1851+
data: {
1852+
left: './b-side-effect.scss',
1853+
right: './g-side-effect',
1854+
},
1855+
},
1856+
{
1857+
messageId: 'unexpectedImportsGroupOrder',
1858+
data: {
1859+
left: './g-side-effect',
1860+
leftGroup: 'unknown',
1861+
right: './a-side-effect.css',
1862+
rightGroup: 'side-effect-style',
1863+
},
1864+
},
1865+
{
1866+
messageId: 'missedSpacingBetweenImports',
1867+
data: {
1868+
left: './a-side-effect.css',
1869+
right: './a',
1870+
},
1871+
},
1872+
],
1873+
},
1874+
],
1875+
},
1876+
)
1877+
})
1878+
15571879
ruleTester.run(
15581880
`${ruleName}(${type}): allows to trim special characters`,
15591881
rule,
@@ -1739,9 +2061,9 @@ describe(ruleName, () => {
17392061
17402062
import a from '.'
17412063
import h from '../../h'
2064+
import './style.css'
17422065
import { j } from '../j'
17432066
import { K, L, M } from '../k'
1744-
import './style.css'
17452067
`,
17462068
options: [
17472069
{
@@ -1832,13 +2154,6 @@ describe(ruleName, () => {
18322154
right: './style.css',
18332155
},
18342156
},
1835-
{
1836-
messageId: 'unexpectedImportsOrder',
1837-
data: {
1838-
left: './style.css',
1839-
right: '../j',
1840-
},
1841-
},
18422157
],
18432158
},
18442159
],
@@ -3277,8 +3592,8 @@ describe(ruleName, () => {
32773592
32783593
import { K, L, M } from '../k'
32793594
import { j } from '../j'
3280-
import h from '../../h'
32813595
import './style.css'
3596+
import h from '../../h'
32823597
import a from '.'
32833598
`,
32843599
options: [
@@ -4953,6 +5268,15 @@ describe(ruleName, () => {
49535268
right: '~/data',
49545269
},
49555270
},
5271+
{
5272+
messageId: 'unexpectedImportsGroupOrder',
5273+
data: {
5274+
left: '~/data',
5275+
leftGroup: 'side-effect',
5276+
right: '~/css/globals.css',
5277+
rightGroup: 'side-effect-style',
5278+
},
5279+
},
49565280
],
49575281
},
49585282
],
@@ -5218,6 +5542,59 @@ describe(ruleName, () => {
52185542
invalid: [],
52195543
})
52205544

5545+
describe(`${ruleName}: checks compatibility between 'sortSideEffects' and 'groups'`, () => {
5546+
let createRule = (
5547+
groups: Options<string[]>[0]['groups'],
5548+
sortSideEffects: boolean = false,
5549+
) =>
5550+
rule.create({
5551+
options: [
5552+
{
5553+
groups,
5554+
sortSideEffects,
5555+
},
5556+
],
5557+
} as Readonly<RuleContext<MESSAGE_ID, Options<string[]>>>)
5558+
let expectedThrownError =
5559+
"Side effect groups cannot be nested with non side effect groups when 'sortSideEffects' is 'false'."
5560+
5561+
it(`${ruleName}: throws if 'sideEffects' is in a non side effects only nested group`, () => {
5562+
expect(() =>
5563+
createRule(['external', ['side-effect', 'internal']]),
5564+
).toThrow(expectedThrownError)
5565+
})
5566+
5567+
it(`${ruleName}: throws if 'sideEffectsStyle' is in a non side effects only nested group`, () => {
5568+
expect(() =>
5569+
createRule(['external', ['side-effect-style', 'internal']]),
5570+
).toThrow(expectedThrownError)
5571+
})
5572+
5573+
it(`${ruleName}: throws if 'sideEffectsStyle' and 'sideEffectsStyle' are in a non side effects only nested group`, () => {
5574+
expect(() =>
5575+
createRule([
5576+
'external',
5577+
['side-effect-style', 'internal', 'side-effect'],
5578+
]),
5579+
).toThrow(expectedThrownError)
5580+
})
5581+
5582+
it(`${ruleName}: allows 'sideEffects' and 'sideEffectsStyle' in the same group`, () => {
5583+
expect(() =>
5584+
createRule(['external', ['side-effect-style', 'side-effect']]),
5585+
).not.toThrow(expectedThrownError)
5586+
})
5587+
5588+
it(`${ruleName}: allows 'sideEffects' and 'sideEffectsStyle' anywhere 'sortSideEffects' is true`, () => {
5589+
expect(() =>
5590+
createRule(
5591+
['external', ['side-effect-style', 'internal', 'side-effect']],
5592+
true,
5593+
),
5594+
).not.toThrow(expectedThrownError)
5595+
})
5596+
})
5597+
52215598
describe(`${ruleName}: allows to use regex matcher`, () => {
52225599
let options = {
52235600
type: 'alphabetical',

‎utils/get-options-with-clean-groups.ts

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
interface GroupOptions {
2+
groups: (string[] | string)[]
3+
}
4+
5+
export const getOptionsWithCleanGroups = <T extends GroupOptions>(
6+
options: T,
7+
): T => ({
8+
...options,
9+
groups: options.groups
10+
.filter(group => group.length > 0)
11+
.map(group =>
12+
typeof group === 'string' ? group : getCleanedNestedGroups(group),
13+
),
14+
})
15+
16+
const getCleanedNestedGroups = (nestedGroup: string[]): string[] | string =>
17+
nestedGroup.length === 1 ? nestedGroup[0] : nestedGroup

‎utils/sort-nodes-by-groups.ts

+37-6
Original file line numberDiff line numberDiff line change
@@ -8,27 +8,58 @@ interface GroupOptions {
88
groups: (string[] | string)[]
99
}
1010

11+
interface ExtraOptions<T extends SortingNode> {
12+
/**
13+
* If not provided, `options` will be used. If function returns null, nodes
14+
* will not be sorted within the group.
15+
*/
16+
getGroupCompareOptions?: (groupNumber: number) => CompareOptions | null
17+
isNodeIgnored?: (node: T) => boolean
18+
}
19+
1120
export let sortNodesByGroups = <T extends SortingNode>(
1221
nodes: T[],
1322
options: CompareOptions & GroupOptions,
23+
extraOptions?: ExtraOptions<T>,
1424
): T[] => {
15-
let nodesByGroupNumber: {
25+
let nodesByNonIgnoredGroupNumber: {
1626
[key: number]: T[]
1727
} = {}
18-
for (let sortingNode of nodes.values()) {
28+
let ignoredNodeIndices: number[] = []
29+
for (let [index, sortingNode] of nodes.entries()) {
30+
if (extraOptions?.isNodeIgnored?.(sortingNode)) {
31+
ignoredNodeIndices.push(index)
32+
continue
33+
}
1934
let groupNum = getGroupNumber(options.groups, sortingNode)
20-
nodesByGroupNumber[groupNum] = nodesByGroupNumber[groupNum] ?? []
21-
nodesByGroupNumber[groupNum].push(sortingNode)
35+
nodesByNonIgnoredGroupNumber[groupNum] =
36+
nodesByNonIgnoredGroupNumber[groupNum] ?? []
37+
nodesByNonIgnoredGroupNumber[groupNum].push(sortingNode)
2238
}
2339

2440
let sortedNodes: T[] = []
25-
for (let groupNumber of Object.keys(nodesByGroupNumber).sort(
41+
for (let groupNumber of Object.keys(nodesByNonIgnoredGroupNumber).sort(
2642
(a, b) => Number(a) - Number(b),
2743
)) {
44+
let compareOptions = extraOptions?.getGroupCompareOptions
45+
? extraOptions.getGroupCompareOptions(Number(groupNumber))
46+
: options
47+
if (!compareOptions) {
48+
sortedNodes.push(...nodesByNonIgnoredGroupNumber[Number(groupNumber)])
49+
continue
50+
}
2851
sortedNodes.push(
29-
...sortNodes(nodesByGroupNumber[Number(groupNumber)], options),
52+
...sortNodes(
53+
nodesByNonIgnoredGroupNumber[Number(groupNumber)],
54+
compareOptions,
55+
),
3056
)
3157
}
3258

59+
// Add ignored nodes at the same position as they were before linting
60+
for (let ignoredIndex of ignoredNodeIndices) {
61+
sortedNodes.splice(ignoredIndex, 0, nodes[ignoredIndex])
62+
}
63+
3364
return sortedNodes
3465
}

0 commit comments

Comments
 (0)
Please sign in to comment.