Support max-line-length in organize imports

Referenced in #22974

By the way, I ran "organize imports" on about 800 files. Works really well other than this problem and it puts imports that were previously on multiple lines onto one... so that will break my linting rule for "max-line-length". Luckily it's easy to write a script to investigate and break them back onto multiple lines 😄

I would say the correct fix here is to let the formatter split lines greater than certain length.

This would be awesome.

So that this:

import { AfterContentInit, Component, ContentChildren, Directive, EventEmitter, HostBinding, Input, ModuleWithProviders, NgModule, OnInit, Optional, Output, QueryList, ViewEncapsulation, forwardRef } from '@angular/core';

becomes this:

import {
  AfterContentInit,
  Component,
  ContentChildren,
  Directive,
  EventEmitter,
  HostBinding,
  Input,
  ModuleWithProviders,
  NgModule,
  OnInit,
  Optional,
  Output,
  QueryList,
  ViewEncapsulation,
  forwardRef
} from '@angular/core';

Thus, respecting max-line-length (tslint).

The auto-importer also breaks the ordered-imports TSLint rule, which specifies how imports should be broken up into groups and how they should be sorted. It would be nice if it were possible to enable/disable various parts of this organize imports command; in my case I want to disable everything except removing unused imports, because everything else is covered by a combination of TSLint and Prettier.

Tom Crockett (@pelotom)

Tom Crockett (@pelotom) that is tracked by #23366

Mohamed Hegazy (@mhegazy)

Mohamed Hegazy (@mhegazy) That issue seems to be specifically about the ordering of named imports within a single statement, but doesn't cover

ordering of different import statements relative to one another
groups of imports

or am I missing something?

which part you are saying conflict with the tslint rule? i thought the problem is that within the same import declaration, named import bindings are ordered differently by tslint (case insensitive) and by tsserver (case sensitive). are there other issues?

Mohamed Hegazy (@mhegazy)

Mohamed Hegazy (@mhegazy) TSlint also allows configuring the order of import statements relative to one another:

You may set the "import-sources-order" option to control the ordering of source imports (the "foo" in import {A, B, C} from "foo").

Possible values for "import-sources-order" are:
-"case-insensitive': Correct order is "Bar", "baz", "Foo". (This is the default.)
-"lowercase-first": Correct order is "baz", "Bar", "Foo".
-"lowercase-last": Correct order is "Bar", "Foo", "baz".
-"any": Allow any order.

And it has an option for configuring groupings of import statements:

You may set the "grouped-imports" option to control the grouping of source imports (the "foo" in import {A, B, C} from "foo").

Possible values for "grouped-imports" are:

false: Do not enforce grouping. (This is the default.)

true: Group source imports by "bar", "../baz", "./foo".

That is, it creates newline-separated groups like this:

import 'a';
import 'b';

import '../a';
import '../b';

import './a';
import './b';

Currently Organize Imports destroys these groupings and mashes them all together.

I see, thanks for sharing. i do not think such level of configurability will be in in organize imports the server supports. i think the best solution here is to not use organizeImports, and instead have tslint-vscode-plugin fix your lint errors on save.

Mohamed Hegazy (@mhegazy)

Mohamed Hegazy (@mhegazy)

i think the best solution here is to not use organizeImports, and instead have tslint-vscode-plugin fix your lint errors on save.

Unfortunately that's not viable for removing unused imports, because the vscode plugin for TSLint can't apply rules which require type checking. Besides that, the TSLint rule for no-unused-variable has myriad problems and Erich Gamma (@egamma) has advised people to use TypeScript's native noUnusedLocals and noUnusedParameters instead. See https://github.com/Microsoft/vscode-tslint/issues/185, https://github.com/Microsoft/vscode-tslint/issues/70.

Is there any chance that a "remove unused imports" command could be added to the language server?

Erich Gamma (@egamma)

apply rules which require type checking

that has nothing to do with type checking. the organize imports does not require full type-checking, nor does it require semantic state outside the current file, it uses the find-all-refs logic.

Besides that, the TSLint rule for no-unused-variables has myriad problems and Erich Gamma (@egamma) has advised people to use TypeScript's native noUnusedLocals and noUnusedParameters instead.

i think we are talking about two different things here. 1. what APIs needed from the TypeScript language service to make tslint better, and 2. what level of configuration is needed in organize imports feature

For 1, we would be happy to add additional APIs to enable tslint to be faster/more efficient. for 2, as i noted, the value of adding more configuration drops fairly quickly vs the value added.

I would say start a tslint issue, and we will talk o the tslint core team about what APIs we can enable to simplify this scenario.

Is there any chance that a "remove unused imports" command could be added to the language server?

There is already a suggestion for that, that is marked as unused (see #22361), and it comes with a quickfix to remove it, with quickFix all option on it. so an IDE could query for suggestions, and apply all fixes on save if that is needed. not specific to imports though, all declarations.

that has nothing to do with type checking. the organize imports does not require full type-checking, nor does it require semantic state outside the current file, it uses the find-all-refs logic.

I'm talking about the TSLint no-unused-variable rule. I have no idea why it claims to require type checking, but it does, and type checking rules cannot be run inside VSCode (see the issues I linked). My point is just that there's no way as of right now to remove unused imports in VSCode with TSLint, which is why I was hoping to be able to use the Organize Imports feature to do it :)

There is already a suggestion for that, that is marked as unused (see #22361), and it comes with a quickfix to remove it, with quickFix all option on it. so an IDE could query for suggestions, and apply all fixes on save if that is needed. not specific to imports though, all declarations.

Great, looks like exactly what I want!

Aleksey-Bykov

Aleksey-Bykov I don't want to remove or make exceptions for the tslint rule if at all possible, but I appreciate that I could and that for some, that may be their preferred solution.

I would like if VS Code could accommodate a tsline line length rule (or rule specified in VS Code) that does not have exceptions for imports, because for now I manually run the command to organize imports then edit for length (as that would not work for organize on save unless I do make exceptions for the tslint rule).

Ryan Cavanaugh (@RyanCavanaugh)

Ryan Cavanaugh (@RyanCavanaugh) i apologize for being a stink bug, this is what i am, this is what i do, didn't mean to pick on you

Aleksey-Bykov

Aleksey-Bykov you've made your preference clear. Some other peoples' preference is for all code to fit within their screen's width to prevent needing to horizontally scroll regardless of whether they are using tslint—having the import statements automatically format would make the editing experience a little nicer for those people because they wouldn't need to fix this every time they run organize imports.

If anything, I think it would be more productive to ask for this to be configurable (which it likely will be if implemented) so you can turn it off.

David Sherret (@dsherret)

David Sherret (@dsherret) i see, ok, although it's not what the original problem is about (and because of the fact it is not), i buy it

Aleksey-Bykov

Aleksey-Bykov I think it's related to the original problem (I'm the one quoted in it). This isn't really tslint related... think about what the motivation for that tslint rule is and transitively that's what the original post is about.

Additionally, the API and tsserver don't care about tslint. This feature if implemented would likely be a setting specified on FormatCodeSettings or its base (see here). Then vscode and other editors can provide that value however they please.

Anyway, this is a bit of a digression... (my apologies)

VSCode Workaround for those who are having problem with format on save + sort import You can save the file, wait for it to be sorted, then you can break the long import line and than use the "Save without Formatting" option of vscode. Hit ctrl+p and type Save without Formatting.

If you're like me, you like Prettier and VS Code but don't like how they clash when handling imports.

I've found a solution for VS Code that's been working very nicely, keeping the super fast formatting speed of Prettier while keeping imports tidy using eslint-plugin-import.

Check out the .eslintrc, .eslintignore, .vscode/settings.json, .prettierrc, and any devDependencies containing "typescript" or "eslint" in .package.json of https://github.com/bduffany/nextjs-app-template

Brandon Duffany (@bduffany)

I spent so much time getting it right (and failed!)... Thank for sharing your config Brandon Duffany (@bduffany)! Your solution seems to work perfectly!

I haven't read through everything and don't get why there isn't an option for this yet. I'm not using prettier (So I think I'm using the default formatter? It's all a bit confusing when you're not completely into this kind of stuff) but es-lint with the typescript plugin. Couldn't we just say that we make a setting (for example: "source.organizeImportsMaxLen": 100) and break imports into one per line? Just provide the option and people can turn it on or off if they please.

Additionally, I'd suggest if you want something different like filing the line and only breaking when the import doesn't fit on it, you should have to use a custom formatter.

What I mean:

import {bla, bla, bla, bla
    bla, bla, bla, bla, bla,
    bla} from './bla';

This is also affecting exports. My multi-line export statement is being turned into one line:

export {
	Core,
	TokenKeyword,
	TokenSymbol,
	Token,
};

is being turned into a single line. Funny thing, export default {...}; does not have the same issue.

I'm also using "source.fixAll": true" if that makes a difference. Hopefully, I got everything correct and didn't make a complete fool of myself :D

Edit:
Why not just add a few more options so everyone can configure this as they pleases:

sortImports
deleteUnusedImports
importLineMode (I'd suggest options like: auto, mutli-line, single-line, manual / off)

Hi, for those who is still struggling on those issues, please try JS/TS Import Sorter which I believe covers all following topics discussed here:

Max-line-length
TSLint/prettier compatible
Groups of imports
Removal of unused imports

If you do find bugs or features not supported, I'm happy to accept open issues as a maintainer.

Thanks!

DoZerg (@daidodo)

DoZerg (@daidodo) Works like a treat well done! It's also really configurable.

Question is, maybe this should work out of the box but it's not up to me to decide this.

Any updates on this?

We ended up just using both organize imports and prettier on save.

I vote for support "one import/export statement per line with trailing comma", like:

import {
  AfterContentInit, <-- one statement per line
  Component,
  ContentChildren,
  Directive, <-- trailing comma, so additions after this line doesn't affect this line
} from '@angular/core';

instead of max-line-length property. This plays nicely with diffing, merging and code review.

Clearly one size doesn't fit all so why even vote? :)

Navigation Menu

Repository navigation

Support max-line-length in organize imports #22991

15 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Support max-line-length in organize imports #22991

Description

Activity

mhegazy commented on Mar 29, 2018

MartinMa commented on May 7, 2018

pelotom commented on May 8, 2018

mhegazy commented on May 8, 2018

pelotom commented on May 8, 2018

mhegazy commented on May 8, 2018

pelotom commented on May 8, 2018

mhegazy commented on May 8, 2018

pelotom commented on May 8, 2018

mhegazy commented on May 8, 2018

pelotom commented on May 8, 2018

15 remaining items

crashmstr commented on Apr 2, 2019

zpdDG4gta8XKpMCd commented on Apr 2, 2019

dsherret commented on Apr 2, 2019

zpdDG4gta8XKpMCd commented on Apr 2, 2019

dsherret commented on Apr 2, 2019

vinagreti commented on Aug 26, 2019

bduffany commented on Apr 3, 2020

laurensnl commented on Apr 4, 2020

Elias-Graf commented on Apr 12, 2020

daidodo commented on Apr 13, 2020

Elias-Graf commented on Apr 13, 2020

tommueller commented on Mar 4, 2021

thinkjson commented on Mar 4, 2021

Bessonov commented on Jan 22, 2022

Rush commented on Jan 22, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions