Skip to content

Commit 0847443

Browse files
yndajasljharb
authored andcommitted
Revert 1fa2971 (breaking group change in order)
1fa2971 changed the way groups work when there is only one, leading to single nested groups being treated as though they were unnested. Prior to this, if you wanted to group e.g. builtin and external imports together at the top and everything else together as a second group, a single nested group was the way to do it [It appears that this change was unintentional][1], and was made to try to fix what seems to be a misunderstanding around nested groups ([#2687]). [The docs][2] continue to suggest that nested groups should be "mingled together" and makes no reference to a single nested group with no other groups being an invalid option This therefore reverts the change to how groups work when there is only one. No documentation change should be necessary given this is already the described behaviour [1]: #2687 (comment) [2]: https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/order.md#groups-array
1 parent e9de30a commit 0847443

File tree

3 files changed

+31
-39
lines changed

3 files changed

+31
-39
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
66

77
## [Unreleased]
88

9+
### Fixed
10+
- [`order`]: revert breaking change to single nested group ([#2854], thanks [@yndajas])
11+
912
### Changed
1013
- [Docs] remove duplicate fixable notices in docs ([#2850], thanks [@bmish])
1114

@@ -1082,6 +1085,7 @@ for info on changes for earlier releases.
10821085

10831086
[`memo-parser`]: ./memo-parser/README.md
10841087

1088+
[#2854]: https://github.com/import-js/eslint-plugin-import/pull/2854
10851089
[#2850]: https://github.com/import-js/eslint-plugin-import/pull/2850
10861090
[#2842]: https://github.com/import-js/eslint-plugin-import/pull/2842
10871091
[#2835]: https://github.com/import-js/eslint-plugin-import/pull/2835
@@ -1887,5 +1891,6 @@ for info on changes for earlier releases.
18871891
[@wtgtybhertgeghgtwtg]: https://github.com/wtgtybhertgeghgtwtg
18881892
[@xM8WVqaG]: https://github.com/xM8WVqaG
18891893
[@xpl]: https://github.com/xpl
1894+
[@yndajas]: https://github.com/yndajas
18901895
[@yordis]: https://github.com/yordis
18911896
[@zloirock]: https://github.com/zloirock

src/rules/order.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -422,10 +422,6 @@ const types = ['builtin', 'external', 'internal', 'unknown', 'parent', 'sibling'
422422
// Example: { index: 0, sibling: 1, parent: 1, external: 1, builtin: 2, internal: 2 }
423423
// Will throw an error if it contains a type that does not exist, or has a duplicate
424424
function convertGroupsToRanks(groups) {
425-
if (groups.length === 1) {
426-
// TODO: remove this `if` and fix the bug
427-
return convertGroupsToRanks(groups[0]);
428-
}
429425
const rankObject = groups.reduce(function (res, group, index) {
430426
[].concat(group).forEach(function (groupItem) {
431427
if (types.indexOf(groupItem) === -1) {

tests/src/rules/order.js

Lines changed: 26 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2753,7 +2753,7 @@ context('TypeScript', function () {
27532753
};
27542754

27552755
ruleTester.run('order', rule, {
2756-
valid: [
2756+
valid: [].concat(
27572757
// #1667: typescript type import support
27582758

27592759
// Option alphabetize: {order: 'asc'}
@@ -2962,7 +2962,31 @@ context('TypeScript', function () {
29622962
},
29632963
],
29642964
}),
2965-
],
2965+
isCoreModule('node:child_process') && isCoreModule('node:fs/promises') ? [
2966+
test({
2967+
code: `
2968+
import express from 'express';
2969+
import log4js from 'log4js';
2970+
import chpro from 'node:child_process';
2971+
// import fsp from 'node:fs/promises';
2972+
`,
2973+
options: [{
2974+
groups: [
2975+
[
2976+
'builtin',
2977+
'external',
2978+
'internal',
2979+
'parent',
2980+
'sibling',
2981+
'index',
2982+
'object',
2983+
'type',
2984+
],
2985+
],
2986+
}],
2987+
}),
2988+
] : [],
2989+
),
29662990
invalid: [].concat(
29672991
// Option alphabetize: {order: 'asc'}
29682992
test({
@@ -3218,39 +3242,6 @@ context('TypeScript', function () {
32183242
// { message: '`node:fs/promises` import should occur before import of `express`' },
32193243
],
32203244
}),
3221-
3222-
test({
3223-
code: `
3224-
import express from 'express';
3225-
import log4js from 'log4js';
3226-
import chpro from 'node:child_process';
3227-
// import fsp from 'node:fs/promises';
3228-
`,
3229-
output: `
3230-
import chpro from 'node:child_process';
3231-
import express from 'express';
3232-
import log4js from 'log4js';
3233-
// import fsp from 'node:fs/promises';
3234-
`,
3235-
options: [{
3236-
groups: [
3237-
[
3238-
'builtin',
3239-
'external',
3240-
'internal',
3241-
'parent',
3242-
'sibling',
3243-
'index',
3244-
'object',
3245-
'type',
3246-
],
3247-
],
3248-
}],
3249-
errors: [
3250-
{ message: '`node:child_process` import should occur before import of `express`' },
3251-
// { message: '`node:fs/promises` import should occur before import of `express`' },
3252-
],
3253-
}),
32543245
] : [],
32553246
),
32563247
});

0 commit comments

Comments
 (0)