format-imports-vscode icon indicating copy to clipboard operation
format-imports-vscode copied to clipboard

Groups with similar starting regex doesn't group correctly

Open Nxt3 opened this issue 4 years ago • 9 comments

If you have custom group rules and both rules have the same start, it will default to the first group in the rules. Expected behavior A clear and concise description of what you expected to happen.

Actual behavior

Given:

import { Component } from '@angular/core';
import { MyService } from '@mymodule/common';

import { SomethingComponent } from './local-component'

will group to:

import { Component } from '@angular/core';
import { MyService } from '@mymodule/common';

import { SomethingComponent } from './local-component'

but I would expect:

import { Component } from '@angular/core';

import { MyService } from '@mymodule/common';

import { SomethingComponent } from './local-component'

OS (please complete the following information): VS Code (please complete the following information):

Version: 1.53.0-insider
Commit: 7b0cfdd04ad530a9c8d782c618afd79290be3d64
Date: 2021-01-27T08:57:30.935Z
Electron: 11.2.1
Chrome: 87.0.4280.141
Node.js: 12.18.3
V8: 8.7.220.31-electron.0
OS: Darwin x64 19.6.0

VS Code settings:

  "tsImportSorter.configuration.groupRules": ["^[@]", "^@mymodule/", {}, "^[.]"],
  "tsImportSorter.configuration.maxBindingNamesPerLine": 0,
  "tsImportSorter.configuration.maxDefaultAndBindingNamesPerLine": 0

Nxt3 avatar Jan 27 '21 18:01 Nxt3

Hi @Nxt3, thanks for the feedback!

Apparently, "^[@]" is too broad so perhaps you need to be more specific about what you want for the 1st group. My best guess is ["^@angular", "^@", {}, "^[.]"].

Speaking of your suggestion, it's hard to decide which group a statement should go when matched by multiple ones. So first one winning seems a simple and reasonable solution. However, I'm happy to hear your opinions.

daidodo avatar Jan 27 '21 19:01 daidodo

The issue with ["^@angular", "^@", {}, "^[.]"] is that it only covers @angular and I'd need to update it every time there is a new import I encounter that . Like in the below:

import { take } from 'rxjs/operators';
import { Component } from '@angular/core';
import { SomethingFactory } from '@thirdpartydep';

import { MyService } from '@mymodule/common';

I'd need to combine several patterns for the first group. I would expect the sorting to happen like:

  • Make first pass and sort with first group
  • Second pass is second group (take stuff from the first group if necessary)
  • etc.

Ideally it would be great to implement grouping based on the location of the import, similar to how vsc-organize-imports does it: https://github.com/alfnielsen/vsc-base/tree/master/vsc-organize-imports#group-options so that grouping rules aren't handled via pattern matching.

Nxt3 avatar Jan 27 '21 20:01 Nxt3

Let me understand it. Is it right that you want:

  1. Everything starting with @mymodule goes to the 2nd group;
  2. Anything else starting with @ goes to the 1st group?

If yes, maybe you can use this: ["^@(?!mymodule/)", "^@mymodule/", {}, "^[.]"]

daidodo avatar Jan 27 '21 21:01 daidodo

Technically not just anything with @xxx; because what would happen to import { take } from 'rxjs/operators'; in that case? It would end up in the third group wouldn't it?

Nxt3 avatar Jan 27 '21 21:01 Nxt3

Yes. So the question boils down to what your grouping criteria are?

daidodo avatar Jan 27 '21 22:01 daidodo

I linked above but maybe options for grouping based on locality? Essentially:

// first group
imports from node_modules

// second group
imports from local modules (like '@mymodule/common')

// third group
imports from relative paths (like '../stuff')

https://github.com/alfnielsen/vsc-base/tree/master/vsc-organize-imports#group-options

Nxt3 avatar Jan 27 '21 22:01 Nxt3

I checked the code and now understand the notions of global and absolute imports.

I agree it's a nice feature to distinguish local and external modules. But given that it needs to probe every import path and consider baseUrl and path alias, I'd need more motivation to bring it to the priority list.

daidodo avatar Jan 27 '21 23:01 daidodo

Need this too. Match order and sort order should be separated.

Current logic:

with rule: [{"regex": "^b"}, {"regex": "^a"}], to decide where import "a-package" go:

  1. try "^b", fail
  2. try "^a", success, the order is 2
  3. do sub-group logic in group 2

Separate logic:

with rule: [{"regex": "^b", "score": 999}, {"regex": "^a", "order": 1}], to decide where import "a-package" go:

  1. try "^b", fail
  2. try "^a", success, the order is 1
  3. do sub-group logic in group 1

There should be a default order value, maybe +Infinity.

GongT avatar Jul 03 '23 23:07 GongT

Need this too. Match order and sort order should be separated.

Current logic:

with rule: [{"regex": "^b"}, {"regex": "^a"}], to decide where import "a-package" go:

  1. try "^b", fail
  2. try "^a", success, the order is 2
  3. do sub-group logic in group 2

Separate logic:

with rule: [{"regex": "^b", "score": 999}, {"regex": "^a", "order": 1}], to decide where import "a-package" go:

  1. try "^b", fail
  2. try "^a", success, the order is 1
  3. do sub-group logic in group 1

There should be a default order value, maybe +Infinity.

GongT avatar Jul 03 '23 23:07 GongT