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

Use same alphabetize props as ESLint sort-imports #1670

Open
Sawtaytoes opened this issue Feb 27, 2020 · 22 comments
Open

Use same alphabetize props as ESLint sort-imports #1670

Sawtaytoes opened this issue Feb 27, 2020 · 22 comments

Comments

@Sawtaytoes
Copy link

Sawtaytoes commented Feb 27, 2020

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
Copy link
Member

ljharb commented Feb 27, 2020

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

@Sawtaytoes
Copy link
Author

Sawtaytoes commented Feb 28, 2020

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
Copy link
Member

ljharb commented Feb 28, 2020

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
Copy link
Author

Sawtaytoes commented Feb 28, 2020

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
Copy link
Member

ljharb commented Feb 28, 2020

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
Copy link
Author

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
Copy link
Member

ljharb commented Feb 28, 2020

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
Copy link
Author

Sawtaytoes commented Mar 2, 2020

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
Copy link
Member

ljharb commented Mar 2, 2020

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
Copy link

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
Copy link

pcarn commented Apr 28, 2020

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

@disovi
Copy link

disovi commented Aug 13, 2020

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
Copy link

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

Same!

@ThaJay
Copy link

ThaJay commented Nov 12, 2020

What I want:

  • imprts get grouped by path (the groups setting)
  • groups get sorted by syntax (the sort-imports memberSyntaxSortOrder setting)
  • syntax groups get sorted alphabetically by path
  • the variables inside a multiple import statement get sorted

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
Copy link
Contributor

silviogutierrez commented Jan 21, 2021

@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
Copy link

ThaJay commented Jan 26, 2021

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
Copy link

AbdealiLoKo commented Jan 21, 2022

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
Copy link
Author

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

@KevinNovak
Copy link

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
Copy link

@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
Copy link

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
Copy link

ThaJay commented Nov 29, 2023

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

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

No branches or pull requests