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

Improve default group regexps to make them easier to copy-paste and re-order, and improve grouping docs #47

Closed
Aemple opened this issue Jul 14, 2020 · 7 comments

Comments

@Aemple
Copy link

Aemple commented Jul 14, 2020

Hi, I have some problems sorting by groups.

First problem

Eslint Config

{
  ...

   rules: {
     "simple-import-sort/sort":  [
        "error",
        {
          groups: [["^\\u0000"], ["^[^.]"], ["^@?\\w"], ["^\\."]],
        },
      ],
    },

  ...
}

import code

    import react from "react";
    import test from "@/components/test";
    import "./style.less";
    import {sort} from "../src"

Expected behavior

    import "./style.less";

    import test from "@/components/test";

    import react from "react";

    import {sort} from "../src"

Actual behavior

    import "./style.less";

    import test from "@/components/test";
    import react from "react";

    import {sort} from "../src"

So I think the defaultGroups need to be changed, The user should be able to change the import order directly by changing the defaultGroups, but this is not possible now, as this will mislead the user。for example, change it like below:

const defaultGroups = [
   // Side effect imports.
   ["^\\u0000"],
   // Packages.
   // Things that start with a letter (or digit or underscore), or `@` followed by a letter.
   ["^@?\\w"],
   // Absolute imports and other imports such as Vue-style `@/foo`.
   // Anything that does not start with a dot.
   ["(^@/[^.])|(^/[^.])"],
   // Relative imports.
   // Anything that starts with a dot.
   ["^\\."],
];

Second problem

if ["^\u0000"] is at the end of the groups

Eslint Config

{
  ...

   rules: {
     "simple-import-sort/sort":  [
        "error",
        {
          groups: [  ["^@?\\w"], ["^\\."],["^[^.]"],["^\\u0000"]],
        },
      ],
    },

  ...
}

** import code **

    import react from "react";
    import test from "@/components/test";
    import "./style.less";
    import {sort} from "../src"

Expected behavior

    import react from "react";

    import {sort} from "../src";

    import test from "@/components/test";

    import "./style.less";

Actual behavior

    import react from "react";

    import {sort} from "../src";

    import "./style.less";
    import test from "@/components/test";

The reason for this problem is that the 【"^[^.]"】 rules and 【["^\u0000"]】 rules coincide

Solution One: change group

       groups: [  ["^@?\\w"], ["^\\."],["^[^.]"],["^\\u0000"]]   =>  groups: [  ["^@?\\w"], ["^\\."],["(^@/[^.])|(^/[^.])"],["^\\u0000"]]

Solution Two: change flatMap function; such as:

         flatMap(itemGroups, (groups) =>
                groups.map((group) => [group, group.regex.exec(source)])
          )
----------------------------------------------------------------------------------
         flatMap(itemGroups, (groups) =>
               groups.map((group) =>  group.regex.source.includes("^\\u0000")
                  ? [group, group.regex.exec(source)]
                  : RegExp("^\\u0000", "u").exec(source)
                  ? [group, null]
                  : [group, group.regex.exec(source)];)
        )

I think it should be possible to change the import position by changing the parameter position in the default groups, but this is not possible now 🤡

Should change the default groups, and Readme documents 🤠🤠🤠

@Aemple
Copy link
Author

Aemple commented Jul 14, 2020

@lydell Please help look at this problem

@lydell
Copy link
Owner

lydell commented Jul 15, 2020

First problem

It looks like you expect "@/components/test" to end up in "^@?\\w"? "@components/test" would have matched, but the slash makes it not match. I don’t know the intention here so I can’t know the correct solution, but changing to "^@/" fixes the issue in the small example at least.

Second problem

You can change "^[^.]" in "^[^.\\u0000]" to fix the issue.

Alternatively, you can change "^\\u0000" into "^\\u0000.*" – that works too (longest match wins).

Comments

Good idea about improving the default groups, so people can reorder those regexps without being mislead!

I will do that in the next release, and look through the docs again.

Question

The suggested change in flatMap – what does it do? And is it even needed if we change the default group regexps?

@Aemple
Copy link
Author

Aemple commented Jul 15, 2020

hi
@lydell

first problem

When ["^[^.]"] is in front of ["^@?\w"] ,Both["^[^.]"] and ["^@?\w"] can capture import react from "react";, So import react from "react";can't be grouped correctly ,
image

["^[^.]"] has problems in both cases, I think["^[^.]"] should be replaced

Question

Not necessary

@lydell
Copy link
Owner

lydell commented Jul 15, 2020

I see. Changing "^@?\\w" to "^@?\\w+" seems to do the trick?

@Aemple
Copy link
Author

Aemple commented Jul 15, 2020

Yes, so the default groups and readme documents need to be changed

The first way
Default groups need to modify "^@?\w" and "^\u0000"

The second way
Modify ["^[^.]"] so that ["^[^.]"] has no intersection with "^@?\w" and "^\u0000"

@lydell lydell changed the title Sort by groups failed Improve default group regexps to make them easier to copy-paste and re-order, and improve grouping docs Jul 15, 2020
@lydell lydell closed this as completed in 52ee218 Nov 7, 2020
@tutods
Copy link

tutods commented Jun 4, 2022

I want exclude a specific import from one group to use into another group.
Is possible?

{
						groups: [
							['^react', '^next', '^recoil', '^@?\\w'],
							['^styles', '^ui/styles', './styles'],
							[
								`^(${folders.join('|')})(/.*|$)`,
								'^\\.'
							],
							['^[^.]']
						]
					}

this are my groups, and I want remove ./styles and use it on second group styles,ui/styles. Is possible?

@lydell
Copy link
Owner

lydell commented Jun 4, 2022

Hi @tutods! I’m not quite following what you’re trying to do. Can you open a new issue, with some more details and examples?

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

No branches or pull requests

3 participants