Issues with replacing @peterjuras's extension

[robyoder]

This is not a drop-in replacement for peterjuras/vsc-sort-imports, as it would need to be if users are expected to migrate to this fork. If these are deliberate design decisions, that's understandable (it's your fork), so feel free to close this, but it can't function as a drop-in replacement without these things changing. I don't know if that was your goal or if Peter simply recommended yours since he was done with his.

That said, here are the problems.

This extension does not place a blank line between groups (or "buckets") of imports. In other words, where Peter's extension would yield the following for the module import style:

import * as _ from "lodash";

import { helperFunc } from "./helpers";

this one produces this:

import * as _ from "lodash";
import { helperFunc } from "./helpers";

That means that migrating to your extension causes every file to change when its imports are sorted, and there is no preference to change that.

In addition, this extension doesn't support loading a style, including a custom style, from the package.json. I defined my own variant of import-sort-style-module which conforms to the TSLint ordered-imports rule while still grouping imports like the original import-sort. My sort style is installed as a node_module in my project and is referenced from my package.json, but it is not respected.

[tomdeman]

I am also seeing issues with ESLint sort-imports since the deprecation and switching.

default ESLint config:

{
    "sort-imports": [2, {
        "ignoreCase": false,
        "ignoreMemberSort": false,
        "memberSyntaxSortOrder": ["none", "all", "multiple", "single"]
    }]
}

peterjuras/vsc-sort-imports (default, passes lint):

import { AllHtmlEntities } from 'html-entities';
import _ from 'lodash';
import fs from 'fs';
import json2xml from 'json2xml';
import oauth from 'oauth';
import qs from 'qs';

amatiasq/vsc-sort-imports (default/by-module-name, fails lint):

import fs from 'fs';
import { AllHtmlEntities } from 'html-entities'; //this line throws lint error for not being alphabetical
import json2xml from 'json2xml';
import _ from 'lodash'; //this line throws lint error for not being alphabetical
import oauth from 'oauth';
import qs from 'qs';

amatiasq/vsc-sort-imports (by-imported-name, fails lint):

import _ from 'lodash';
import fs from 'fs';
import json2xml from 'json2xml';
import oauth from 'oauth';
import qs from 'qs';
import { AllHtmlEntities } from 'html-entities'; //this line throws lint error for not being alphabetical

[amatiasq]

This is not a drop-in replacement for peterjuras/vsc-sort-imports, as it would need to be if users are expected to migrate to this fork.

No, it's not. As explained in this comment

I created this fork months ago to add an option so users can have different types of sorting, it was set as the default option long before this project was announced as unmaintained and to change it now will break it for all users who are using the new version since it was published.

---

I don't know if that was your goal or if Peter simply recommended yours since he was done with his.

You can see the full story here. After I created the fork I opened a backwards compatible PR for Peter's repository but it was never merged. So it's more like the second case.

---

This extension does not place a blank line between groups (or "buckets") of imports.

That means that migrating to your extension causes every file to change when its imports are sorted, and there is no preference to change that.

In addition, this extension doesn't support loading a style, including a custom style, from the tsconfig.json.

I guess supporting custom style would solve this issue, when I forked this I wasn't aware of this feature. I will check if I find time to implement it but if you're in a hurry a PR would be the best option for you.

@tomdeman I guess custom styles would solve your issue too, right?

[tomdeman]

thanks for the quick response.

A custom style could work. However, I would be creating a custom style to conform with the eslint config for the sort-import rule. I would assume this would be a popular use case. Making me question why that would not be the default style for this extension, or at least offered as one of the built in style options. Maybe it's worth a discussion?

I'd like to spend a little more time looking into exactly what ESLint is expecting too. Based on the examples I posted, I have my own questions about the what it is expecting as alphabetical, as well.

I must admit, I have not spent much time researching, as I just recently ran into this after getting the deprecated alert and making the switch. I just think it would be great to have a standard that conforms with the popular linters. It makes it easy to roll out and enforce with a team just by taking advantage of auto sort on save.

but only if the linter's rules make sense, as well.

thanks

[ferdaber]

This extension is essentially a wrapper around the original sort-imports package. The original sort-imports package allows the use of custom styles by defining them in your package.json file.

Since this is an extension it would be feasible to add an extension option in VSCode that essentially achieves the same effect, but it would require the extension user to have that custom style in their node_modules directory (and indirectly, package.json) for the extension to work properly.

Note that this is close to how the ESLint extension works, it will attempt to use the eslint package in the user's node_modules directory before using a global package.

[ChiefORZ]

Essentially i think, we need to go back to the original sort-imports from @peterjuras , because it already included passing a custom style. And secondly just created another custom style which incorporates @amatiasq alphabetic ordering

[amatiasq]

Hey @robyoder @tomdeman @ferdaber those are valid concerns so in order to prevent this to be a bigger problem a new major version has been released.

This will rollback a few features back to the state of Peter's extension. I hope this help you.

Also now that I have a better understanding of the import-sort tool I've migrated the previously default sorting style to it's own NPM module as Peter recommended me. I hope @ChiefORZ this is not what you mean you did otherwise we would have been doing the same work here.

Now that it's back to a simpler state I hope community could help improve this tool since sadly that was all the time I had for personal development for now.

I'll close this thread since it addresses multiple different issues. I'd encourage you to open separated issues for each use case you find from now on so we can work and discuss them one by one.

Thank you for you feedback guys!

[robyoder]

@amatiasq, I'm sorry that my post came across as inconsiderate. I appreciate the effort you and Peter put in to these projects. And I know that you're free to implement your extension however you'd like. The only problem here was that Peter's was deprecated and pushed users to yours, and that was frustrating when it wasn't the same thing.

I did not feel the need to submit a PR when it wasn't clear whether you intended this extension to be equivalent to Peter's. And I was happy to continue using Peter's since he removed the deprecation popup. If I had an issue in the future, I could fork it myself or find another extension that was an equivalent fork. But I thought that if this project was meant to be equivalent/backwards-compatible, these issues should be addressed.

So thank you for addressing them and providing the ability to use custom import styles. I'm happy that you see the value of using import-sort's power directly. 🙂 👍

[tomdeman]

@robyoder - perfectly put. thanks to all, especially the devs for their time

[amatiasq]

Thank you guys, this is the first time I handle a "widely" used open source project and I was a little bit overwhelmed by the result. I asked Peter to delegate his extension on this one but clearly I didn't consider the whole scope of it. But I'm happy I could help you now that that extension is unmaintained.