Support for maxLineLength setting in vscode-settings

[Nxt3]

I've seen it recommended a few places here to use the maxLineLength: 0 setting when running into issues with prettier.

Would it be possible to set this in vscode settings as opposed to the importSorter config?

[daidodo]

@Nxt3, thanks for the feedback!

Could you provide links about the issues and recommendations? I need to understand them better before making decisions.

Basically, maxLineLength is not in VS Code settings because it's covered by Prettier and EditorConfig. VS Code settings are generally not recommended IMHO because it's not portable in team works. But I'm open to different perspectives.

[Nxt3]

Sure: #36 (comment) and #29 (comment)

[daidodo]

Ah, I see where you're from.

Are you having the same issue with #36? If yes, maxLineLength won't help you as commented. The real solve for that would be either disabling formatting or compatible with Prettier.

[Nxt3]

Yes I'm running into the same problem. If there was a way to have it format so that everything is always on one line and then let prettier handle it that would probably be the fix.

[daidodo]

That's essentially running prettier after this extension, which you said couldn't achieve.

[Nxt3]

The other person was posting the issue was saying that if you had the following:

import { FakeImportWithVeryLooongName } from './components/fake-import-file-name-very-loooong/fake-import-file-name-very-loooong';

ts-import-sorter would format it like so:

import {
  FakeImportWithVeryLooongName
} from './components/fake-import-file-name-very-loooong/fake-import-file-name-very-loooong';

whereas prettier would leave it like this:

import { FakeImportWithVeryLooongName } from './components/fake-import-file-name-very-loooong/fake-import-file-name-very-loooong';

so that would imply that there is some extra processing happening in ts-import-sorter to make that happen, right?

[daidodo]

Yes, this extension pre-calculates the line length and if it exceeds the limit, it will wrap the statement.
To be honest, I don't think prettier is following the limit fully but a compatible fall-back can be implemented.

[Nxt3]

That's correct: prettier does not format imports in that way; it's left as one-line regardless of line length.

https://prettier.io/docs/en/rationale.html#imports

[daidodo]

Thanks for the reference, that's helpful!

So what I recommend now is implementing the compatible behaviour with prettier, if it's found. Otherwise wrap the line.
What do you think?

And this shouldn't be a big change so it's more than welcome if you'd like to do it in case you're interested.

[Nxt3]

Any chance you could point me to which files to look at? If I found some time I wouldn't mind taking a stab.

[daidodo]

Thanks!
Perhaps the first step is to take a look at CONTRIBUTING.md.
The prettier config is read here so you know if it's found.
And the start point of composing an import is here.

One note is that you might want to check whether the following are wrapped or not by prettier:

import 'a-veeeeeeery-looooooooooong-veeeeeeery-looooooooooong-script';
import A = require('b-veeeeeeery-looooooooooong-veeeeeeery-looooooooooong-module');

As this extension WILL wrap them.

[karlhorky]

This is great, compatibility with Prettier would be awesome! Look forward to this change!

[daidodo]

@Nxt3, have you started anything on this thread? Otherwise I'm going to pick it up soon. Thanks!

[Nxt3]

@daidodo I haven't had a chance 🙃

[daidodo]

No worries, I'll pick it up then.

There are some other issues and more in the TODO list if you still want to get your feet wet.

[daidodo]

Prettier Study

Did a few experiments in Prettier Playground

Seems Prettier doesn't wrap an long import/export declaration except:

• There are more than one names, and
• There is a {}

When wrapping, there is ONE name per line.

So the extension needs the following to be compatible:

• Config:
  • maxBindingNamesPerLine: 0
  • maxDefaultAndBindingNamesPerLine: 0
  • maxNamesPerWrappedLine: 1
  • maxExportNamesPerLine: 0
• Do NOT wrap in the following cases:

  import 'a-veeeeeeery-looooooooooong-veeeeeeery-looooooooooong-script';
  import A from 'aaaaaaaaaaaaaaaaaa'
  import { XXX } from 'xdfdfdfdfdfdf'
  import * as AA from 'dfefefefeffe'
  import A = require('b-veeeeeeery-looooooooooong-veeeeeeery-looooooooooong-module');
  import B, * as C from 'bdaefadfadsfadgadf';
  
  export {} from 'aaaaaaaaaaaaaaaaa';
  export { A } from 'aaaaaaaaaaaaaaaaa';

Recommended Solution

Add a new option wrappingStyle that is:

• An object with the following options:
  • maxBindingNamesPerLine
  • maxDefaultAndBindingNamesPerLine
  • maxNamesPerWrappedLine
  • maxExportNamesPerLine
• Or 'prettier'.

When merging config, wrappingStyle will be replaced as whole instead of merging each option in the object.

Cons:

• Backward incompatible;

Alternatives

1. An option for not breaking lines for certain cases

Add an option, e.g. prettierCompatible: boolean. When it's true, don't wrap lines in the above certain cases:

Cons:

• Users need to figure out other configs to mimic Prettier behaviour, e.g. maxBindingNamesPerLine: 0.

2. An option to do all the mimic

Add an option, e.g. prettierWrapping: boolean. When it's true, follow the Prettier line wrapping style and ignore maxBindingNamesPerLine etc. settings.

Cons:

• maxBindingNamesPerLine etc. settings become confusing.

Discussions

1. Is it useful to add prettierCompatible: boolean to the object in the recommended solution?
   No. Currently, I can't think of a use case which needs prettierCompatible: true without Prettier.

@Nxt3 and @karlhorky, please tell me if you have any thoughts!
Thanks!

Edit:

More studies on Prettier

It doesn't consider long comments when wrapping:

import { A, B } from 'a'; // long long comment
export { A, B }; // long long comment
export { A, B } from 'a'; // long long comment

import type and export type are the same as above.

[Nxt3]

That was a nice write-up! Thanks for putting that together.

I'm a fan of Option 2 with prettierWrapping. 😎

[daidodo]

Option 2 was my favourite too as a straightforward solution. But the nitpicking maxXXX settings made me a bit uncomfortable.

I don't have the data but generally I don't expect people use maxXXX settings out of Prettier compatibility issues. If that's true, I think the backward incompatibility of the recommended solution wouldn't be a problem.

[karlhorky]

Yeah, although I don't know the project too well, I would also vote for the recommended solution (breaking change, but less confusing).

There's been so many times that dealing with confusing APIs has led to hours of pain for me and those around me. Breaking changes mean everyone is affected, but less pain overall (and this extension is relatively "young" still, so breaking changes hurt less).

[daidodo]

@Nxt3 and @karlhorky, I've release v7.0.0 with the recommended solution.
Please check it out and tell me if you have any questions.

Thanks!

[karlhorky]

Just installed it, seems to be good so far!

One thing is that it does say that "tsImportSorter.configuration.wrappingStyle": "prettier" is an "Unknown Configuration Setting" - I guess that's just temporary?

[daidodo]

Good catch! Fix is coming.

[daidodo]

@karlhorky Please try v7.0.1.

[karlhorky]

Looks good to me, thanks!

I've added it to the System Setup Guide for our coding bootcamp at @upleveled and we'll use it together with the students and report any issues :)

[daidodo]

Thanks! I'll close the issue but feel free to reopen or create a new one if you have any questions.

[Nxt3]

Forgot to leave some appreciation! This turned out beautifully; makes this extension the first one to get sorting / format correct. Fantastic job and thanks again!