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: "type" group is not enough #2441

Open
iliubinskii opened this issue Apr 24, 2022 · 9 comments · May be fixed by #2615 or #3104
Open

import/order: "type" group is not enough #2441

iliubinskii opened this issue Apr 24, 2022 · 9 comments · May be fixed by #2615 or #3104

Comments

@iliubinskii
Copy link

Currently, the rule offers the following groups:
"builtin", "external", "internal", "unknown", "parent", "sibling", "index", "object", "type"

To me, having single "type" group for all type imports is not enough.

I can import types from "builtin" modules, from "external" module and so on.

So, it would be logical to have type counterpart for each group, i.e.:
"build" -> "built-type"
"external" -> "external-type"
and so on

@ljharb
Copy link
Member

ljharb commented Apr 26, 2022

That's a combinatorial explosion of categories, unfortunately.

I can see some kind of object form of each category that includes types: true - and then, any types not already swept up by another category would be in "type".

@csandman
Copy link

I came to mention a similar thing however I think customizing the type imports might be a bit too much.

Type imports are really their own thing, while every other import group is based on the import source, the Type imports are all categorized based on the import type keyword. I feel like sorting types among themselves with the same ordering that is applied to the rest of the categories would probably be an ideal solution to keep things looking consistent, otherwise internal types which use relative imports will usually be placed above external types, even if you have external types listed first.

@ljharb
Copy link
Member

ljharb commented Apr 26, 2022

That's what it does currently, ftr.

@csandman
Copy link

It sorts based on the order of the import groups? That's not what I'm seeing.

These are my settings for the rule:

"import/order": [
  "warn",
  {
    "groups": [
      "builtin",
      "external",
      "internal",
      "parent",
      "sibling",
      "object",
      "type"
    ],
    "newlines-between": "never",
    "alphabetize": {
      "order": "asc",
      "caseInsensitive": false
    }
  }
],

And this is how its sorting my types on a run of eslint --fix:

import { Box } from "@chakra-ui/layout";
import React from "react";
import type { Size, SizeProps } from "../types";
import type { SystemStyleObject } from "@chakra-ui/system";
import type { ReactElement } from "react";
import type {
  ContainerProps,
  GroupBase,
  IndicatorsContainerProps,
  ValueContainerProps,
} from "react-select";

I would expect the type import from "../types" to go to the bottom:

import { Box } from "@chakra-ui/layout";
import React from "react";
import type { SystemStyleObject } from "@chakra-ui/system";
import type { ReactElement } from "react";
import type {
  ContainerProps,
  GroupBase,
  IndicatorsContainerProps,
  ValueContainerProps,
} from "react-select";
import type { Size, SizeProps } from "../types";

@ljharb
Copy link
Member

ljharb commented Apr 26, 2022

That's what it should do, then :-) a PR with failing test cases is appreciated.

@7nik
Copy link

7nik commented Dec 1, 2022

It looks like type imports get sorted in reverse order of groups.
E.g. I get this:

    import type { Paginator } from "../../utils/NMApi";
    import type NM from "../../utils/NMTypes";
    import type { Actors } from "../TradeWindow.svelte";
    import type { Writable } from "svelte/store";

    import { onDestroy, createEventDispatcher, getContext } from "svelte";
    import { writable } from "svelte/store";
    import NMApi from "../../utils/NMApi";
    import tip from "../actions/tip";
    import Avatar from "../elements/Avatar.svelte";
    import Button from "../elements/Button.svelte";
    import Icon from "../elements/Icon.svelte";
    import FiltersMenu from "./FiltersMenu.svelte";
    import PrintDetails from "./PrintDetails.svelte";

with the following rule options

"import/order": [
    "error",
    {
        groups: [
            "type",
            "builtin",
            "external",
            "internal",
            "parent",
            "sibling",
            "index",
            "object",
        ],
        alphabetize: {
            order: "asc",
            caseInsensitive: true,
        },
        warnOnUnassignedImports: false,
    },
],

@ljharb
Copy link
Member

ljharb commented Dec 1, 2022

@7nik would you be able to make a PR with a failing test case?

@7nik
Copy link

7nik commented Dec 1, 2022

add test cases to https://github.com/import-js/eslint-plugin-import/blob/main/tests/src/rules/order.js ?
can try in a few days

@ljharb
Copy link
Member

ljharb commented Dec 1, 2022

Yep, thanks! Much obliged; I can much more easily add a fix onto a failing PR :-)

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