import-js / eslint-plugin-import

ESLint plugin with rules that help validate proper imports.
MIT License
5.57k stars 1.57k forks source link

`import/order` option alphabetize.order doesn't preserve group order after 2.26.0 version #3077

Closed renarsvilnis closed 1 month ago

renarsvilnis commented 1 month ago

Hi, I have noticed when updating eslint dependencies on an existing config that post 2.26.0 version (it's an ancient version, but has been a blocker on our end for upgrading this particular dependency) that it apparently breaks sort order of imports within ["parent", "sibling", "index"] group (see screenshot visual diff after upgrade and autofix).

Screenshot 2024-10-01 at 21 09 17

👇🏼 Noticed that in other places the sort didn't make sense to me as it seemed to go ["sibling", "relative", "sibling"] 🤷🏼

Screenshot 2024-10-01 at 21 13 03

Seems like changes by adding orderImportKind within https://github.com/import-js/eslint-plugin-import/compare/v2.26.0...v2.27.0#diff-2825fcc427c846fe18d501930e1a3d83be722027f1f643801afdcc2c255dd4a8R4 has caused the issue, which seems unexpected as the default orderImportKind: "ignore" should have kept previous ordering.

Update #1 Digged into the code a bit and as far as I understand that the following lines split A/B import paths into segments by / and wort by each segment accordingly. But that would also mean that first relative traverse parts e.g. .. vs . would always sort to . > .. ignoring the type of the import, right?

Update #2 Found a open discussion about the same problem https://github.com/import-js/eslint-plugin-import/issues/2682


alphabetize.order: "asc" no longer sorts imports within "sub-group" as before where it first sorted parent imports, than sibling, index, but threats all group imports as a whole. Before that was a powerful feature to mentally group dependencies in a file from outside in in contextual blocks of what we control and what is related to the file.

My config for import/order:

 "import/order": [
    "error",
    {
      "newlines-between": "always",
      pathGroupsExcludedImportTypes: ["builtin"],
      groups: [
        ["builtin", "external"],
        ["internal"],
        ["parent", "sibling", "index"],
        ["object"],
        /**
         * NOTE: There is also "type" group, but we are not specifying so it groups depending on its import path
         * and then get alphabetized depending on path
         */
      ],
      alphabetize: {
        order: "asc",
        caseInsensitive: true,
      },
    },
  ],

I found a possible workaround by splitting the parent into its own group. But it is unfavorable as it would touch pretty much all files in the project.

Temporary workaround is to:

 "import/order": [
    "error",
    {
      "newlines-between": "always",
      pathGroupsExcludedImportTypes: ["builtin"],
      groups: [
        ["builtin", "external"],
        ["internal"],
-         ["parent", "sibling", "index"],
+         ["parent"],
+         ["sibling", "index"],
        ["object"],
        /**
         * NOTE: There is also "type" group, but we are not specifying so it groups depending on its import path
         * and then get alphabetized depending on path
         */
      ],
      alphabetize: {
        order: "asc",
        caseInsensitive: true,
      },
    },
  ],
renarsvilnis commented 1 month ago

~Closing issue as found a solution that had to flip the arguments to get it working.~

```diff alphabetize: { - order: "asc", + order: "ignore", + orderImportKind: "asc", caseInsensitive: true, }, ```

Reopened as while no errors I had with the config (see details) on existing code, the lack of sorting actually prevents new code from getting properly sorted in the first place.

renarsvilnis commented 1 month ago

Update (again...) Found a open discussion about the same problem https://github.com/import-js/eslint-plugin-import/issues/2682