import-js / eslint-plugin-import

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

Use same alphabetize props as ESLint sort-imports #1670

Open Sawtaytoes opened 4 years ago

Sawtaytoes commented 4 years ago

ESLint's native sort-imports does the sorting I want, but doesn't care about the grouping of imports. On the other hand, import/order gives me the ordering I want, but not the powerful alphabetization sort from sort-imports.

For sort-imports, this is the config I use:

{
  'sort-imports': [
    'warn',
    {
      ignoreCase: true,
      memberSyntaxSortOrder: [
        'single',
        'all',
        'multiple',
        'none',
      ],
    },
  ],
}

I'd like to do something similar with import/order:

{
  alphabetize: {
    caseInsensitive: true,
    memberSyntaxSortOrder: [
      'single',
      'all',
      'multiple',
      'none',
    ],
    order: 'asc',
  },
}

I would like this functionality because I'm grouping them differently than the default:

{
  groups: [
    [
      'builtin',
      'external',
    ],
    [
      'index',
      'parent',
      'sibling',
    ],
  ],
}

Here's an example I'd expect to see:

import PropTypes from 'prop-types'
import React from 'react'

import useMediaQuery from './useMediaQuery'
import useMediaQueryState from './useMediaQueryState'
import { * as actions } from './actions'
import { breakpointsList } from './breakpoints'
import './styles.css'
ljharb commented 4 years ago

import/order has alphabetize, does that not do what you want?

Sawtaytoes commented 4 years ago

It does alphabetize, but the ordering is based on the from string (from what I can tell), not the characters of the variables you're importing.

That's why I was suggesting similar functionality to what's in the memberSyntaxSortOrder from the official ESLint import sorting. It allows you to say where the { should be ordered and where * and "no from" statements get ordered.

It also optionally has the ability to sort single-line imports as well so this would get alphabetically sorted in the deconstruction as well:

import { one, two } from './numbers'
ljharb commented 4 years ago

Ah, thanks for clarifying.

I don't see value in sorting by the identifier names - especially since you can have multiples. What would you sort by in this:?

import { a as z, b, d }, c from 'e';
Sawtaytoes commented 4 years ago

In your example, I'd keep c in front and sort the inside has you have it. I never even thought the default export could be after the deconstruction.

// Sorted deconstruction
import c, { a as z, b, d } from 'e';
ljharb commented 4 years ago

Right, but then you're sorting by the exported names, but the importing default name. If the name the importer chooses is the primary bit, then i'd expect as z to win out over a.

Sawtaytoes commented 4 years ago

That is a really good point!

My main concern is being able to quickly find things. If everything's alphabetical, it's in the same order each time no matter the component. It also needs to be something I could reasonably fix without

I have no opinion on sorting by a or z in a as z as long as it's easy to spot when skimming imports.

Are we in agreement on the other parts or is there more contention?

ljharb commented 4 years ago

What's the use case tho? Like, what are you hoping to quickly find that you're not using grep ("find" in your editor), or "go to definition" jumping from a usage site?

Sawtaytoes commented 4 years ago

Alphabetizing anything where order doesn't matter makes it easier to skim unimportant parts of PRs. There's also improved compression ratio because there are more situations where we have repeat code. And with code-consistency, it becomes easier to find and replace.

My main thing for me is having similar functionality to the official ESLint rule, but separating node_modules/ from local imports like eslint-plugin-import provides.

Would it help if I provided the PR myself? I completely understand if that's the determining factor here.

ljharb commented 4 years ago

I appreciate that, and providing the PR certainly helps! While I strongly agree with the sentiment in making it easier to skip unimportant parts of PRs (reducing diff churn, eg), to me it's more important that the lint rule make sense on its own. Just sorting things for the sake of sorting them doesn't seem particularly compelling to me.

gmiklich commented 4 years ago

Just playing devil's advocate here, but couldn't you apply your rationale about "sorting things just for the sake of sorting" to the entire idea of grouping and sorting imports in general? If I'm looking to modify an existing import or looking over a PR that made such a modification, it seems just as nice to be able to find an import group as it is to find a imported member/identifier within a single statement.

There are rules to order object keys, vars, and as was pointed out here, rules in tslint or other eslint plugins to order the imported members. And this plugin orders and groups imports, so why would you not order the identifiers within? As for your example, I also don't particularly care about which way you choose to order, as long as it's consistent.

pcarn commented 4 years ago

I'd also like to have the imports inside the group sorted.

disovi commented 4 years ago

just two cents from TypeScript world, in TSLint there was a rule ordered-imports and there was an option named-imports-order: "lowercase-first". As soon as TSLint is dead and everyone somehow migrates to eslint it would be nice to have similar option for compatibility.

jeverick commented 4 years ago

I'd also like to have the imports inside the group sorted.

Same!

ThaJay commented 4 years ago

What I want:

I have the following config:

    'sort-imports': [
      'warn',
      {
        ignoreCase: true,
        memberSyntaxSortOrder: ['multiple', 'single', 'all', 'none']
      },
    ],
    'import/order': [
      'warn',
      {
        'groups': ['builtin', 'external', 'parent', 'sibling', 'index'],
        'pathGroups': [
          {
            pattern: 'react*',
            group: 'external',
            position: 'before',
          },
          {
            pattern: '@react*',
            group: 'external',
            position: 'before',
          },
        ],
        'pathGroupsExcludedImportTypes': ['builtin'],
        'newlines-between': 'never',
      },
    ],

example:

import React from 'react'
import anotherNodeModule from 'another-node-module'
import otherNodeModule from 'other-node-module'
import Something from '../something'
import {xSibling, ySibling, zSibling} from './lettered-siblings'
import sibling from './sibling'

This makes sort-imports complain anotherNodeModule should come before React. If memberSyntaxSortOrder would be included in this plugin, I could disable sort-imports.

It does not complain if I set 'newlines-between': 'always', and sort-imports allowSeparatedGroups: true, but then there would be quite a few import blocks that consist of a single import line and a newline, so I would like to avoid that if at all possible.

silviogutierrez commented 3 years ago

@ThaJay : this is pretty close to what I want. Fortunately in my case, the rules do work without conflict, but I lose auto fixing for the memberSyntaxSortOrder part.

Basically, I'm trying to emulate Python's isort. But isort has an easier time because you cannot combine default/namespace/named imports into one statement. It's all or nothing.

My platonic ideal:

import * as fs from "fs";

import * as reactivated from "reactivated";
import {Router, browserHistory} from "react-router";
import React from "react";

import * as typography from "@client/shared/typography";
import Login from "@client/scenes/Login";
import {useSceneSubmitHandler} from "@client/scenes/utils";

import * as constants from "./constants";
import * as style from "./style";
import {Thunk} from "./reducer";

First broken up by chunks (builtin, external, internal, relative). Then within that, grouped into import syntax. Then alphabetized.

Currently, this plugin can automate all but the last step. If I turn on alphabetize, I get this:

import * as fs from "fs";

import React from "react";
import {Router, browserHistory} from "react-router";
import * as reactivated from "reactivated";

import Login from "@client/scenes/Login";
import {useSceneSubmitHandler} from "@client/scenes/utils";
import * as typography from "@client/shared/typography";

import * as constants from "./constants";
import {Thunk} from "./reducer";
import * as style from "./style";

And that to be looks a lot worse. Moreover, I then need to disable memberSyntaxSortOrder otherwise I get a conflict.

So currently I can either: fully automate but lose grouping syntax, or almost fully automate but need to manually group syntax.

ThaJay commented 3 years ago

Compared to my previous config posted here, if I add ignoreDeclarationSort: true, to 'sort-imports' and add more pathGroups like so it works well enough:

'pathGroups': [
  {
    pattern: 'react',
    group: 'external',
    position: 'before',
  },
  {
    pattern: 'react-native',
    group: 'external',
    position: 'before',
  },
  {
    pattern: 'react*',
    group: 'external',
    position: 'before',
  },
  {
    pattern: '@react*',
    group: 'external',
    position: 'before',
  }
]
AbdealiLoKo commented 2 years ago

Came here looking for the ability to sort import within a single import: import { b, a } from './alphabet' to import { a, b } from './alphabet'

Any luck on the PR @Sawtaytoes or was there any suggested workarounds for this ?

Sawtaytoes commented 2 years ago

No luck on my end. Someone will need to patch it.

KevinNovak commented 2 years ago

I have the following setup with "sort-imports" and "import/order" and they seem to be working well together:

{
    "import/order": [
        "error",
        {
            "alphabetize": {
                "caseInsensitive": true,
                "order": "asc"
            },
            "groups": [
                ["builtin", "external", "object", "type"],
                ["internal", "parent", "sibling", "index"]
            ],
            "newlines-between": "always"
        }
    ],
    "sort-imports": [
        "error",
        {
            "allowSeparatedGroups": true,
            "ignoreCase": true,
            "ignoreDeclarationSort": true,
            "ignoreMemberSort": false,
            "memberSyntaxSortOrder": ["none", "all", "multiple", "single"]
        }
    ]
}
AbdealiLoKo commented 2 years ago

@KevinNovak Thanks a lot - that got it working for me. I didn't notice the ignoreMemberSort: true config in sort-imports. Looks like that will allow import/order to do it's thing but add the member sorting from sort-import.

javier-sierra-sngular commented 11 months ago

I encourage the development of this new feature. I believe it is necessary. I don't think it's right to have to use two plugins to achieve it.

ThaJay commented 11 months ago

Loads has changed in the meantime. So I thought it may be useful for someone to share my current config. There's good grouping and from paths are sorted by depth. You get member sorting by default. The groups config is not very readable but that's their api.

    'import/first': 'error',
    'import/newline-after-import': 'error',
    'simple-import-sort/exports': 'error',
    'simple-import-sort/imports': [
      'error',
      {
        groups: [
          ['^\\u0000'],
          ['(?=^.)react(?!.)', '(?=^.)react(?!.).*\\u0000$', '(?=^.)react-native(?!.)', '(?=^.)react-native.*\\u0000$'],
          ['^node:', '^node:.*\\u0000$'],
          ['^react', '^@?\\w', '^react.*\\u0000$', '^@?\\w.*\\u0000$'],
          ['(?<!\\u0000)$', '(?<=\\u0000)$'],
          ['^\\.', '^\\..*\\u0000$'],
        ],
      },
    ],

This is what it looks like: image image