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

More control over how type specifiers are handled #147

Closed
angrybacon opened this issue Feb 9, 2024 · 14 comments
Closed

More control over how type specifiers are handled #147

angrybacon opened this issue Feb 9, 2024 · 14 comments
Assignees

Comments

@angrybacon
Copy link

angrybacon commented Feb 9, 2024

Is your feature request related to a problem?

I would like the plugin not to split my import statements when they have a type specifier. For context, let's assume types are set to be sorted first, then everything else.

import { type C, B, type A } from 'module';

Becomes

import type { A, C } from 'module';
import { B } from 'module';

Where I expected

import { B, type A, type C } from 'module';

Or at the very least if sorting within an import statement is not supported I expected my code not to be modified at all (I don't want duplicated import paths)

Describe the solution you'd like

My goals for my React TypeScript codebase with this plugin:

  • Merge duplicated paths
  • Prefer type specifiers
  • Not use ESLint fixable rules which I'm trying to migrate from (very expensive on CI)

Here are some examples of how I wish the sorting happened

Merging duplicate paths
import type { A } from 'one';
import { B, C } from 'one';
import { E } from 'two';
import { B, C, type A } from 'one';
import { E } from 'two';
Sorting types last
import { type B, D, C, type A } from 'one';
import { E } from 'two';
import { C, D, type A, type B } from 'one';
import { E } from 'two';

Describe alternatives you've considered

If enforcing type specifiers all the time proves too difficult, then I'd prefer forbidding type specifiers entirely even if that means duplicated paths.

Pulling out type imports
import { type B, D, C, type A } from 'one';
import { E } from 'two';
import type { A, B } from 'one';
import { C, D } from 'one';
import { E } from 'two';
Enforcing type imports over specifiers
import { type B, type A } from 'one';
import { E } from 'two';
import type { A, B } from 'one';
import { E } from 'two';
@IanVS
Copy link
Owner

IanVS commented Feb 9, 2024

Can you please share your prettier configuration?

@angrybacon
Copy link
Author

{
  "importOrder": [
    "<TYPES>",
    "<BUILTIN_MODULES>",
    "<THIRD_PARTY_MODULES>"
  ],
  "importOrderTypeScriptVersion": "5",
  "plugins": ["@ianvs/prettier-plugin-sort-imports"],
  "proseWrap": "always",
  "singleQuote": true
}

@IanVS
Copy link
Owner

IanVS commented Feb 9, 2024

If you do not include <TYPES> in the importOrder array, they will not be broken out separately from the value imports. Can you try removing that and see if it does what you want?

@angrybacon
Copy link
Author

Hello again, sorry for the late reply, I finally found some time to test your suggestion.

Removing <TYPES> from the order list does prevent statements to be split 👍 thank you for that. For posterity, with the following ESLint rule:

{
  "import/consistent-type-specifier-style": ["error", "prefer-inline"]
}

I get to enforce inline specifiers. Running prettier afterwards fixes the last artifacts (extraneous spaces usually).

However, I still have 1 remaining issue: I see support for importOrderCaseInsensitive was dropped and this is a big issue for me since I don't wish to change the way I'm sorting my imports. To me there's value in sorting uppercase first: constants, constructors and functions are sorted in that order.

Is there a chance the option comes back? What is the reasoning behind its deprecation?

Thanks in advance 🙏

@m-salman-afzal
Copy link

@IanVS @angrybacon I had the same issue with duplicate imports. As per discussion, if you remove <TYPES> , then you won't have ordering in types. But if you include them, there is chance for duplicate imports. If there was an option that we can mix and match so that duplicate imports gets merged, all other remain separated due to <TYPES>.

OR

Maybe there is an Eslint rule modification of no-duplicate-imports that say if the duplicate import is of only type, then ignore that warning or error. Is this feasible? How it can be achieved?

@IanVS
Copy link
Owner

IanVS commented Feb 21, 2024

As per discussion, if you remove <TYPES>, then you won't have ordering in types. But if you include them, there is chance for duplicate imports.

Can you give some examples of what you mean here?

@m-salman-afzal
Copy link

With <TYPES>:

import {C} from "modA";

import type {A} from "modA"; //duplicate import
import type {B} from "modB";

Without <TYPES>:

import type {D} from "modD"; // types can be written anywhere

import {C, type A} from "modA"; // this could also be in duplicate form where type is from separate import

import type {B} from "modB";

Optional behavior with <TYPES>:

import {C, type A} from "modA";

import type {B} from "modB";
import type {D} from "modD";

WIth the last option, there is a mix and match where types are indeed organized but if there is a duplicate import, only that gets merged.

@IanVS
Copy link
Owner

IanVS commented Feb 21, 2024

For your "// types can be written anywhere" example, that's not quite true. They will be sorted according to the import specifier, just like value imports, and in fact will be combined together if needed. So "// this could also be in duplicate form where type is from separate import" isn't accurate either.

So, as far as I can tell, your last set of examples is indeed the current behavior. If not, can you please share a "before" and "after" formatting example, along with the prettier configuration that you're using?

@m-salman-afzal
Copy link

Yeah, that was my bad. Here is .prettierrc.json

{
    "arrowParens": "always",
    "bracketSameLine": true,
    "bracketSpacing": false,
    "embeddedLanguageFormatting": "auto",
    "endOfLine": "crlf",
    "htmlWhitespaceSensitivity": "css",
    "jsxSingleQuote": false,
    "printWidth": 120,
    "proseWrap": "always",
    "quoteProps": "as-needed",
    "tabWidth": 4,
    "trailingComma": "none",
    "semi": true,
    "singleQuote": false,
    "singleAttributePerLine": false,
    "useTabs": false,
    "plugins": ["@ianvs/prettier-plugin-sort-imports"],
    "importOrderParserPlugins": ["typescript", "jsx", "decorators-legacy"],
    "importOrderTypeScriptVersion": "5.0.0",
    "importOrder": [
        "<BUILT_IN_MODULES>",
        "",
        "<THIRD_PARTY_MODULES>",
        "",
        "^@server/(.*)$",
        "",
        "^@routes/(.*)$",
        "",
        "^@middlewares/(.*)$",
        "",
        "^@controllers/(.*)$",
        "",
        "^@entities/(.*)$",
        "",
        "^@valueObjects/(.*)$",
        "",
        "^@validations/(.*)$",
        "",
        "^@transformers/(.*)$",
        "",
        "^@domain/(.*)$",
        "",
        "^@appUtils/(.*)$",
        "",
        "^@application/(.*)$",
        "",
        "^@repositories/(.*)$",
        "",
        "^@infraConfig/(.*)$",
        "",
        "^@infraUtils/(.*)$",
        "",
        "^@infraServices/(.*)$",
        "",
        "^@infrastructure/(.*)$",
        "",
        "^@logger/(.*)$",
        "",
        "@typings/(.*)$",
        "",
        "^[./]",
        "",
        "<TYPES>",
        ""
    ]
}

this makes ordering as follows:

import {injectable} from "tsyringe";

import BaseRepository from "@repositories/BaseRepository";

import {File} from "@infrastructure/Database/Models/File";

import {SEARCH_FILE_REPOSITORY_FIELDS} from "./Shared/Query/FieldsBuilder";
import {FileQueryBuilder} from "./Shared/Query/FileQueryBuilder";

import type {TFilterFile} from "./Shared/Query/FileQueryBuilder"; // this is duplicate import
import type {FileEntity} from "@entities/File/FileEntity";
import type {IFileRepository} from "@entities/File/IFileRepository";
import type PaginationOptions from "@infraUtils/PaginationOptions";

without last <TYPES>, I have:

import {injectable} from "tsyringe";

import type {FileEntity} from "@entities/File/FileEntity";
import type {IFileRepository} from "@entities/File/IFileRepository";

import BaseRepository from "@repositories/BaseRepository";

import type PaginationOptions from "@infraUtils/PaginationOptions";

import {File} from "@infrastructure/Database/Models/File";

import {SEARCH_FILE_REPOSITORY_FIELDS} from "./Shared/Query/FieldsBuilder";
import {FileQueryBuilder, type TFilterFile} from "./Shared/Query/FileQueryBuilder";

no doubt, now the duplicate is removed since we have single import but now even the imports that are type only are now mixed inbetween. And what I was thinking if we could have something like:

import {injectable} from "tsyringe";

import BaseRepository from "@repositories/BaseRepository";

import {File, type TFilterFile} from "@infrastructure/Database/Models/File"; // only this one is being mixed because it has both type and simple import

import {SEARCH_FILE_REPOSITORY_FIELDS} from "./Shared/Query/FieldsBuilder";
import {FileQueryBuilder} from "./Shared/Query/FileQueryBuilder";

// all other type only imports are organized below as indicated by <TYPES>
import type {FileEntity} from "@entities/File/FileEntity";
import type {IFileRepository} from "@entities/File/IFileRepository";
import type PaginationOptions from "@infraUtils/PaginationOptions";

@angrybacon
Copy link
Author

@IanVS The original issue is solved for me but its title is broad enough that discussion with @m-salman-afzal can continue here. Would you rather I file a new issue to track this specifically:

However, I still have 1 remaining issue: I see support for importOrderCaseInsensitive was dropped and this is a big issue for me since I don't wish to change the way I'm sorting my imports. To me there's value in sorting uppercase first: constants, constructors and functions are sorted in that order.

Is there a chance the option comes back? What is the reasoning behind its deprecation?

@IanVS
Copy link
Owner

IanVS commented Feb 22, 2024

@m-salman-afzal so if I understand correctly, you want a way to move the type-only imports somewhere, without impacting the type + value imports. If so, I'm sorry but I don't think that's a style that we'll be able to support. We try to be a bit flexible with the importOrder options, but we have to be somewhat opinionated (that's the whole philosophy of Prettier, after all). And to me, moving type imports sometimes but not others, depending on other imports from the same file, is unnecessarily confusing. I think instead, you could just disable the no-duplicate-imports eslint rule and put all of your type imports where you want them. The rule is not really helpful if you're using this prettier plugin, because this plugin will handle combining imports whenever possible, so it's not really helping to avoid any issues.

@IanVS
Copy link
Owner

IanVS commented Feb 22, 2024

@angrybacon yes, a new issue about that would be good. We can use that to track interest and see if there are others who feel the same way. I'm reluctant to re-add options, but will consider it.

@IanVS IanVS closed this as completed Feb 26, 2024
@vincerubinetti
Copy link

vincerubinetti commented Sep 27, 2024

I have a sort of related issue.

I want to enforce a consistent type import style, one way or the other. I.e. always have type imports on a separate line from value imports, or always combine them when possible.

I know the ESLint import plugin has a rule just for this, but I don't really want to install it. Every other rule in that package is either useless/not desirable to me, or is already covered by TypeScript/ESLint/Prettier. It seems silly to install a package for one rule; especially a rule that I admit I'm just being anal about and isn't critical at all.

I also already have typescript-eslint installed, which has this rule that almost does what I want... If you have no type specifier on a type import and you auto-fix, it can fix the import to your preferred style, but it wont flag any existing type imports as errors. They seem very resistant to making a change to allow only one style or the other (also see here).

Is there any way to use this plugin to achieve what I'm after?

For example, for these imports, I'd like to split the "cytoscape" value and type imports into their own lines, but right next to each other:

import { useEffect, useMemo, useRef } from "react";
import cytoscape, { type Core, type Css, type EdgeSingular, type NodeSingular } from "cytoscape";
import { truncate } from "lodash";
import { getColorMap } from "@/util/color";
import { lerp } from "@/util/math";
import classes from "./Network.module.css";

I have the following import order:

"^react",
"^[a-zA-Z]",
"^@[a-zA-Z]",
"^@/",
"^/",
"^./",
"^../"

I tried adding a <TYPES> duplicate below each one of those lines, but that (predictably, now that I think about it) results in:

import { useEffect, useMemo, useRef } from "react";
import cytoscape from "cytoscape";
import { truncate } from "lodash";
import type { Core, Css, EdgeSingular, NodeSingular } from "cytoscape";
import { getColorMap } from "@/util/color";
import { lerp } from "@/util/math";
import classes from "./Network.module.css";

@IanVS
Copy link
Owner

IanVS commented Sep 27, 2024

@vincerubinetti I don't think that's possible to do with the options we have today. Would you like to open a feature request with a clear before/after example of what you're looking for?

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

4 participants