From 0847443b0a6d8273a862960e51ffcf478f636abf Mon Sep 17 00:00:00 2001 From: Ynda Jas Date: Wed, 9 Aug 2023 12:15:50 +0100 Subject: [PATCH] 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]: https://github.com/import-js/eslint-plugin-import/issues/2687#issuecomment-1671038558 [2]: https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/order.md#groups-array --- CHANGELOG.md | 5 ++++ src/rules/order.js | 4 --- tests/src/rules/order.js | 61 +++++++++++++++++----------------------- 3 files changed, 31 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 96dbd8c10..bc7da43bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange ## [Unreleased] +### Fixed +- [`order`]: revert breaking change to single nested group ([#2854], thanks [@yndajas]) + ### Changed - [Docs] remove duplicate fixable notices in docs ([#2850], thanks [@bmish]) @@ -1082,6 +1085,7 @@ for info on changes for earlier releases. [`memo-parser`]: ./memo-parser/README.md +[#2854]: https://github.com/import-js/eslint-plugin-import/pull/2854 [#2850]: https://github.com/import-js/eslint-plugin-import/pull/2850 [#2842]: https://github.com/import-js/eslint-plugin-import/pull/2842 [#2835]: https://github.com/import-js/eslint-plugin-import/pull/2835 @@ -1887,5 +1891,6 @@ for info on changes for earlier releases. [@wtgtybhertgeghgtwtg]: https://github.com/wtgtybhertgeghgtwtg [@xM8WVqaG]: https://github.com/xM8WVqaG [@xpl]: https://github.com/xpl +[@yndajas]: https://github.com/yndajas [@yordis]: https://github.com/yordis [@zloirock]: https://github.com/zloirock diff --git a/src/rules/order.js b/src/rules/order.js index 6f70db263..44d25be63 100644 --- a/src/rules/order.js +++ b/src/rules/order.js @@ -422,10 +422,6 @@ const types = ['builtin', 'external', 'internal', 'unknown', 'parent', 'sibling' // Example: { index: 0, sibling: 1, parent: 1, external: 1, builtin: 2, internal: 2 } // Will throw an error if it contains a type that does not exist, or has a duplicate function convertGroupsToRanks(groups) { - if (groups.length === 1) { - // TODO: remove this `if` and fix the bug - return convertGroupsToRanks(groups[0]); - } const rankObject = groups.reduce(function (res, group, index) { [].concat(group).forEach(function (groupItem) { if (types.indexOf(groupItem) === -1) { diff --git a/tests/src/rules/order.js b/tests/src/rules/order.js index 2a44aa06a..a6a8735a6 100644 --- a/tests/src/rules/order.js +++ b/tests/src/rules/order.js @@ -2753,7 +2753,7 @@ context('TypeScript', function () { }; ruleTester.run('order', rule, { - valid: [ + valid: [].concat( // #1667: typescript type import support // Option alphabetize: {order: 'asc'} @@ -2962,7 +2962,31 @@ context('TypeScript', function () { }, ], }), - ], + isCoreModule('node:child_process') && isCoreModule('node:fs/promises') ? [ + test({ + code: ` + import express from 'express'; + import log4js from 'log4js'; + import chpro from 'node:child_process'; + // import fsp from 'node:fs/promises'; + `, + options: [{ + groups: [ + [ + 'builtin', + 'external', + 'internal', + 'parent', + 'sibling', + 'index', + 'object', + 'type', + ], + ], + }], + }), + ] : [], + ), invalid: [].concat( // Option alphabetize: {order: 'asc'} test({ @@ -3218,39 +3242,6 @@ context('TypeScript', function () { // { message: '`node:fs/promises` import should occur before import of `express`' }, ], }), - - test({ - code: ` - import express from 'express'; - import log4js from 'log4js'; - import chpro from 'node:child_process'; - // import fsp from 'node:fs/promises'; - `, - output: ` - import chpro from 'node:child_process'; - import express from 'express'; - import log4js from 'log4js'; - // import fsp from 'node:fs/promises'; - `, - options: [{ - groups: [ - [ - 'builtin', - 'external', - 'internal', - 'parent', - 'sibling', - 'index', - 'object', - 'type', - ], - ], - }], - errors: [ - { message: '`node:child_process` import should occur before import of `express`' }, - // { message: '`node:fs/promises` import should occur before import of `express`' }, - ], - }), ] : [], ), });