Group for first party packages

[Lexicality]

We have a module alias called web for our local code (like @ in #4) and I'd like it to be sorted into it's own group like this:

import React from "react"

import { Foo } from "web/components/foo"

import { Bar } from "./bar"

import/order supports this by doing a bunch of magic with Webpack parsing, but just having a list of first party packages as a config variable would be a lot easier.
Thanks

[lydell]

Hi!

Like mentioned in #4 (comment), I’d recommend choosing an alias that does not look like an npm package name. I think that is less confusing as well. Would that work for you?

[Lexicality]

Not really, no. Our project is very big and has a lot of people on it and I only recently managed to convince everyone that using the web/ prefix is a good idea.
I don't think going back to them again and proposing we rewrite 400+ files to use @/components instead of web/components (especially when the file system is src/web/components) would be a popular move.

[lydell]

Thanks for your reply. I understand that it would be inconvenient to make another such big change and have to re-teach everyone to use for example @ instead of web.

On the other hand – if we did add an option for this you would still end up with 400+ files where the sorting changed?

I really want this plugin to stay free of options for as long as possible, because as soon as we add one it feels like opening the flood gates to more. And I’m not sure trying to parse webpack/Rollup/TypeScript/whatever configs is going to be sustainable.

¯\_(ツ)_/¯

[Lexicality]

There's a bit of a difference between me adding an additional rule to the automatic eslint --fix command that runs as a pre-commit hook to progressively improve the codebase and me convincing people to change how they write code (again). 😛

I entirely appreciate you not wanting to add more complexity to this plugin, and if you don't want to add it I will probably end up writing my own plugin. 🙂

FWIW I think parsing any kind of config is way overkill. A firstPartyPackages option in eslint would do the job perfectly well.

[lydell]

Hmm … could you add a custom little ESLint rule that autofixes from web to the new prefix then? Like import { Foo } from "web/components/foo" to import { Foo } from "~web/components/foo" or something maybe?

If you do write your own plugin then feel free to share it here!

[Lexicality]

Well, admittedly using ~web would match what you have to do inside stylesheets 🤔 (though knowing Webpack, we'd probably end up with url("~~web/...") in them after the change)

Ideally I'd want a plugin that just alphabetises imports by block and then let a different one (eg import/order) make the actual blocks, but I guess with the unknown order that eslint will apply rules in, that wouldn't be very stable.

[ystarlongzi]

Allow custom prefixes, this is very helpful for developers, hope to support this feature.

@lydell

[heygrady]

Normally I use eslint-import-resolver-babel-module with eslint-plugin-import. That allows it to support this case. Of course, people use eslint-plugin-simple-import-sort because the import sorting PR for eslint-plugin-import has not merged (import-js/eslint-plugin-import#1360)

Would be amazing if eslint-plugin-simple-import-sort supported import/resolver settings for resolving these types of custom module setups.

  settings: {
    'import/resolver': {
      'babel-module': {
        root: ['./src/'],
        extensions: ['.js', '.jsx', '.ts', '.tsx', '.d.ts'],
      },
  },

[lydell]

@heygrady Don’t you think it’s confusing that some npm-package-looking imports actually resolve to local code? What’s your thought on my recommendation so far to use for example @/ as a prefix?

[ystarlongzi]

Don’t you think it’s confusing that some npm-package-looking imports actually resolve to local code?

Use the resolve.alias feature of webpack is very useful, otherwise you might import to some common code in the project like this:

import xx from '../../../utils/xx'

Actually, because of the webpack support resolve.alias feature, the code maybe like this:

import xx from '@utils/xx'

What’s your thought on my recommendation so far to use for example @/ as a prefix?

Suggest expose the configuration to allow developers to modify.

[lydell]

Personally, I’ve always bit the bullet and used those super ugly ../../../ imports because it works out of the box with all tools and all editors that teammates use.

IMO, @utils/xx is not good either, because it looks like a scoped npm package (like @babel/core).

What’s your thought on my recommendation so far to use for example @/ as a prefix?

Suggest expose the configuration to allow developers to modify.

Sorry, that’s not what I meant. What I meant is – if I say "The solution is to use a non-ambiguous prefix such as @/" (@/utils/xx) – how would you respond? Would you consider changing to that? If not – why not? Etc. I’m trying to get a more complete picture of people’s situations. "We’ve always done it that way" is not enough for me to make this plugin worse (IMO) by adding configuration that (IMO) is not needed.

[heygrady]

I think the issue is that it's a very common practice. For instance, absolute imports are supported by create-react-app.

There are some caveats with absolute imports (like name collisions with an existing npm package). However, it's not really valuable to bike-shed what people should be doing. The important bit here is that people are using absolute imports. It's supported by eslint, babel, typescript, rollup and webpack.

There is a non-zero set of people who use eslint and babel, use absolute imports for perfectly valid reasons, and who would also like to have their imports sorted.

It's worth noting that your plugin appears to do the "right thing" with camelCasePaths.

import { foo } from 'absolute' // <-- in the wrong spot
import { bar } from 'npm-package'

import { baz } from 'absoluteCamelCase' // <-- in the right spot

import { foo } from './relative'

It's also worth noting that eslint-plugin-import supports absolute imports (if you supply a resolver). This whole conversation would be mute if import-js/eslint-plugin-import#1360 ever landed.

[lydell]

Those are very good points.

[theKashey]

Keep in mind - absolte imports are 100% "native" for TypeScript.

[lydell]

@theKashey Interesting! Can you expand on that? Is there a link to some documentation?

[theKashey]

• long story - https://www.typescriptlang.org/docs/handbook/module-resolution.html
• short story - tsconfig.json support paths, which are more or less equal to webpack-aliases, but are the language feature.

[lydell]

Grouping idea:

1. Side effect imports.
2. Node.js core modules and modules specified in dependencies and devDependencies of the package.json of your project.
3. Everything else.
4. Relative imports.

So, instead of using validate-npm-package-name to decide what should go in group 2, we use a list of Node.js core modules and look in the closest package.json.

Do you think it would work?

EDIT: Hmm, probably not. I have actually aliased "react" to "preact" in a project, which means that "preact" is present in package.json while "react" isn’t.

Maybe we could look for directory names inside node_modules up the directory tree instead.

[theKashey]

Maybe we could look for directory names inside node_modules up the directory tree instead.

Would not for the preact case. However, react could be one of Node.js core modules, just name them core modules.

[lydell]

Ah, good point. Looking inside node_modules wouldn’t work either in the case of aliases.

I won’t add react to “core modules”.

[theKashey]

"Well known modules"?
Actually it could be addressed from another point of view - "simple modules". Like react or path is a simple name, components/Button - is a complex. However @material-ui/Button, and @UI/Button would end in a one group. Which is not a bug, but could be a feature.

Absolute path usually are complex - components/Button or redux/state/list, while "packages" are usually simple - react, react-redux. However - there are exception from both sides.

[lydell]

“Well known modules” won’t solve the problem. Users can alias anything.

“Simple modules” unfortunately won’t work either. Some npm packages let you import from fp-ts/lib/Either and stuff.

Keep the ideas coming!

Some rough ideas I have:

• Re-use the resolver stuff from eslint-plugin-import.
• Let the user provide a function that lets you do whatever they you with the grouping. It might be a little annoying writing that function, but oh well.
• Wait for somebody to write the Ultimate Resolver Package™ and use that.

[lydell]

What if we added a function option like this:

// .eslintrc.js
module.exports = {
  // ...
  rules: {
    "simple-import-sort/sort": ["error", {
      transform: (from, filepath) => {
        return from;
      },
    }],
  }
};

• from is the string after from in imports:

  import x from "..."
                 ^^^

• filepath is the absolute path to the file that the from comes from:

  /home/you/projects/some/file.js
  

• The return value is what eslint-plugin-simple-import-sort should use instead of the original from value when sorting.

This way you could for example inspect the passed from value and return "@/" + from in some cases to move stuff to another group.

So. If this existed – would it be useful to you? Would it be too much work implementing such a function? Feel free to post your transform function here for the sake of discussion.

[lydell]

Another idea is to add support for tsconfig.json and jsconfig.json, like create-react-app. If you use TypeScript you probably already have a tsconfig.json. If you use JavaScript you’ll have to create a jsconfig.json if you use non-standard stuff, which is probably a good idea anyway.

[lydell]

Or maybe I’m just over thinking this, and we should just add an option that is an array of package names, like @Lexicality suggested in the first place.

[lydell]

I’ve made up my mind.

TypeScript is a big thing these days. If you use “absolute imports” with TypeScript, you have to set it up in tsconfig.json, right? Otherwise TypeScript won’t understand your imports and give errors. So it would be really nice if eslint-plugin-simple-import-sort Just Works with TypeScript out of the box.

What about if you don’t use TypeScript? Well, the easiest thing for the plugin is to go with the jsconfig.json approach. Same thing, just another file name. As a bonus your imports will then work in VSCode as well. And there’s nothing stopping other editors and tools from reading jsconfig.json either. create-react-app already supports it, as mentioned.

Pros:

• Works out of the box with TypeScript.
• Adds no options to eslint-plugin-simple-import-sort, which makes it easier to say no to options in the future.
• Lets several tools use the same import config.

Cons:

• A little more complicated to implement than a simple option. But still doable.

---

This is the plan.

I think that we only need to look at baseUrl and paths in tsconfig.json/jsconfig.json. Correct me if I’m wrong!

I’m thinking that the logic will be something like this:

1. For each linted file:

   1. If the file name ends with .ts or .tsx, look for the closest tsconfig.json.
   2. Otherwise look for the closest jsconfig.json.

2. For each import in the linted file:

   1. See if a paths pattern matches the from part. If it does, try to figure out if it points to something in node_modules or not. This decides which group to put the import in.

   2. Otherwise, if there’s a baseUrl – find that directory and look into it. If the from part starts with something in that directory, treat the import as first party.

Here’s a good explanation on how baseUrl and paths really work:

https://github.com/microsoft/TypeScript/blob/ed152b7b0696a297b0d61f041497115476ff6b72/src/compiler/moduleNameResolver.ts#L719-L762

[theKashey]

No objections.

But it would be suuuper good to extract ts/jsconfig resolution logic into the separate package, so other libraries and solutions could reuse it. (could be done as a postprocess)

[lydell]

Yeah, I’m currently researching if there are any packages already that do this. Maybe this one: https://github.com/dividab/tsconfig-paths

[lydell]

Status update: Last time I looked at this, I couldn’t find any module that supports tsconfig.json and jsconfig.json. But maybe we don’t need to support jsconfig.json at all, because tsconfig.json might work for JS too: See prettier/prettier#6702 (comment) and onwards.

[entropitor]

@lydell Something good to now: I'm using absolute imports in typescript but I don't use the paths feature but the baseUrl feature so I reference everything as src/.... Would this be supported as well?

Why not using #18 (comment) and allow aliasing certain modules in the config for this plugin?

In the past I've used https://github.com/galooshi/import-js and I found that to be really good 😄

[lydell]

@entropitor Thanks for letting me know about baseUrl as well. If TypeScript allows that we should support it.

[lydell]

I have a thing coming up that will allow customizing this. I’m still interested in supporting TypeScript out of the box, but I’ll open a new issue for that.

[Lexicality]

Hi, sorry for not taking part in the discussion, I've been extremely busy lately.
I'm mostly in favour of using tsconfig.json, but as I mentioned before our project is very big and very old and has a lot of weird stuff in it - along with a mixture of Typescript and Javascript files.
Our tsconfig.json looks a bit like this:

{        
    "allowJs": true,
    "baseUrl": "media",
    "paths": {
        "*": ["./*", "./web/lib/*"]
    },
}

With a file structure a bit like this:

media/
    img/
    blah/
    stuff/
    web/
        modules/
            aaaa.js
            bbbb.ts
            etc.js
        components/
            foo.ts
            bar.js
            example-file.ts
        lib/
            vendored-lib1/
                index.js
            vendored-lib2.js

And example-file.ts looking a bit like this:

import React from "react";
import lib1 from "vendored-lib1";
import lib2 from "vendored-lib2";

import { Foo } from "web/components/foo";
import aaa from "web/modules/aaa";

import { Bar } from "./bar";

I'm not completely against our vendored libs becoming 1st party packages - but I'd sort of prefer them not to if possible.

Sorry for being unhelpful 😬

[lydell]

You’ll be able to put your vendored libs anywhere you want with my upcoming thing.

[lydell]

v5.0.0 has now been released with the groups option. See custom grouping.

Example:

{
  "rules": {
    "simple-import-sort/sort": [
      "error",
      {
        "groups": [
          ["^\\u0000"],
          ["^@?\\w"],
          ["^(web|vendored-lib)(/.*|$)"],
          ["^\\."]
        ]
      }
    ]
  }
}

The automatic tsconfig.json idea is tracked in: #31