Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

import/order: treat type imports like regular imports? #2070

Closed
alumni opened this issue May 14, 2021 · 11 comments
Closed

import/order: treat type imports like regular imports? #2070

alumni opened this issue May 14, 2021 · 11 comments

Comments

@alumni
Copy link

alumni commented May 14, 2021

How can we treat type imports like a regular import (i.e. the behavior before v2.23.0)? I would expect the following import order to be fine:

import type { Dictionary } from 'lodash';
import { isEmpty } from 'lodash';

import { something } from './relative';

... but the first line gets highlighted:

There should be no empty line within import group

Also, eslint is now running forever, so I can't automatically fix this.

Rule config:

{
  "import/order": [
    "warn",
    {
      "alphabetize": { "order": "asc" },
      "groups": [
        ["builtin", "external"],
        "internal",
        ["parent", "sibling", "index"],
        "object"
      ],
      "newlines-between": "always",
      "pathGroups": [
        {
          "pattern": "*.json",
          "group": "object",
          "patternOptions": { "matchBase": true }
        }
      ]
    }
  ]
}
@geraintwhite
Copy link
Contributor

I think the way you'd do that is add type to your first group array.

@alumni
Copy link
Author

alumni commented May 14, 2021

I think the way you'd do that is add type to your first group array.

That won't work in this case:

import type { Dictionary } from 'lodash';
import { isEmpty } from 'lodash';

import { something } from './relative';
import type { TestType } from './relative';

With 2.22.1 the above code will pass, but with 2.23.0 it will fail.

@ljharb
Copy link
Member

ljharb commented May 14, 2021

That has "type A, value A, value B, type B", so I'd expect that to fail regardless of how you configure it - ie, it should be "type A, value A, type B, value B", or "type A, type B, value A, value B", or the reverse of either.

@fernandopasik
Copy link
Contributor

fernandopasik commented May 15, 2021

After 2.23.0 with this example

import type { PackageJson } from 'type-fest';
import Generator from 'yeoman-generator';

is throwing an error

2:1  error  `yeoman-generator` import should occur before import of `type-fest`  import/order

Wouldn't it be ok since the alphabetical sorting is right?

update: forgot to mention I'm using the rule with the airbnb config, so there's not special treatment of the type group

'import/order': ['error', { groups: [['builtin', 'external', 'internal']] }],

@alumni
Copy link
Author

alumni commented May 15, 2021

@ljharb v2.22.1 was not enforcing any order for value and type imports, they were treated identically. v2.23 introduced this breaking change which sorts type imports separately from value imports. Currently, the imports are sorted like this:

import { isEmpty } from 'lodash';

import { something } from './relative';

import type { TestType } from './relative';
import type { Dictionary } from 'lodash';

... which is also not right according to your own expectation.

Since this is a minor version update, it would have been nice to leave the old behavior unchanged. It is a bit late for that now, but it would still be nice to have to ability to sort type and value imports together - either "type A, value A, value B, type B" or "type A, value A, type B, value B".

@ljharb
Copy link
Member

ljharb commented May 15, 2021

@alumni yes, I agree with that.

We'll need to restore that default behavior, and only differentiate type imports via an option.

@merrywhether
Copy link

merrywhether commented May 15, 2021

That has "type A, value A, value B, type B", so I'd expect that to fail regardless of how you configure it - ie, it should be "type A, value A, type B, value B", or "type A, type B, value A, value B", or the reverse of either.

For the version where types are mixed within groups, it would be nice if ^ was actually enforced. We have a bunch of

import type A from 'a';
import A from 'a';
import B from 'b';
import type B from 'b';

and it would be great to have a more strict ordering if the library is going to start being type-import aware. For reference, @typescript-eslint/consistent-type-imports extracts type imports and places them directly above their corresponding regular imports (though it has no on-going opinion after the initial extraction).

@mikestopcontinues
Copy link

mikestopcontinues commented May 16, 2021

Ideal for me:

  • If type is not listed in groups, sort them right above the matching imports. (Or below, or whatever.)
  • If type is listed, sort all type imports at that location AND sort that group in the same way as the overall group structure, sans line-breaks.
  • Do not let import/first warn for any type imports being out of order regarding other imports. (Which it's currently doing, if you add type to groups.)

This reverts the old behavior for people with old configs, but allows newer configs to leverage the coolness of a type group.

nihalgonsalves added a commit to moia-oss/eslint-prettier-typescript-config that referenced this issue May 17, 2021
2.23 brought a number of breaking changes, including:

- import/order `type` sorting: import-js/eslint-plugin-import#2070
- `import/no-extraneous-dependencies`: import-js/eslint-plugin-import#2065
@ahnpnl
Copy link

ahnpnl commented May 17, 2021

I got the same issue when upgrading from 2.22.1 to 2.23.2, our CI failed https://github.com/kulshekhar/ts-jest/runs/2598048887?check_suite_focus=true

@fernandopasik
Copy link
Contributor

Thanks @ljharb for restoring the behaviour! Is there going to be a release soon?

@ljharb
Copy link
Member

ljharb commented May 21, 2021

#2092

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

7 participants