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

feat: add organizeImportsMode option #140

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,26 @@ const plugin = {
category: 'OrganizeImports',
description: 'Skip destructive code actions like removing unused imports.',
},
organizeImportsMode: {
type: 'choice',
default: 'All',
category: 'OrganizeImports',
description: 'Mode for organizing imports',
choices: [
{
value: 'All',
description: 'Sort, combine and remove unused imports',
},
{
value: 'SortAndCombine',
description: 'Only sort and combine imports',
},
{
value: 'RemoveUnused',
description: 'Only remove unused imports',
},
],
},
},
parsers: {
babel: withOrganizeImportsPreprocess(babelParsers.babel),
Expand Down
17 changes: 15 additions & 2 deletions lib/organize.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,14 @@ const { getLanguageService } = require('./get-language-service');
*/
module.exports.organize = (
code,
{ filepath = 'file.ts', organizeImportsSkipDestructiveCodeActions, parentParser, parser },
{
filepath = 'file.ts',
organizeImportsSkipDestructiveCodeActions,
// @ts-ignore
organizeImportsMode = 'All',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you really need to set a default value here, since we're already setting the default in the plugin options?

Also what is the @ts-ignore ignoring? 🙃

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the default value is unnecessary here, this one could be delete.

parentParser,
parser,
},
) => {
if (parentParser === 'vue') {
// we already did the preprocessing in the parent parser, so we skip the child parsers
Expand All @@ -24,7 +31,13 @@ module.exports.organize = (
const languageService = getLanguageService(parser, filepath, code);

const fileChanges = languageService.organizeImports(
{ type: 'file', fileName: filepath, skipDestructiveCodeActions: organizeImportsSkipDestructiveCodeActions },
{
type: 'file',
fileName: filepath,
skipDestructiveCodeActions: organizeImportsSkipDestructiveCodeActions,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
skipDestructiveCodeActions: organizeImportsSkipDestructiveCodeActions,

I assume we can skip this now that we set mode based on it, since the property is deprecated anyway.

// @ts-ignore
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, why a ts-ignore?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the problem of use enum type in javascript code, since the languageService.organizeImports only accept mode type as OrganizeImportsMode, we must use something like OrganizeImportsMode.All to configure it, and it was imported from the typescript code, that's not a good idea. I couldn't find any other way to solve this problem, the simple way is just ignore it.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok we can just type case it instead. i'll try to finish this PR soon, sorry somehow was too busy lately 🫣

mode: organizeImportsSkipDestructiveCodeActions ? 'SortAndCombine' : organizeImportsMode,
},
{},
{},
)[0];
Expand Down
2 changes: 2 additions & 0 deletions prettier.d.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
declare module 'prettier' {
interface Options {
organizeImportsSkipDestructiveCodeActions?: boolean;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sry i didn't mean dropping the deprecated option entirely. just when calling languageService.organizeImports, to only pass mode there, like

languageService.organizeImports(
  // ...
  mode: organizeImportsMode ?? (organizeImportsSkipDestructiveCodeActions ? 'SortAndCombine' : undefined),
  // ...
)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, restored and updated.

organizeImportsMode?: 'All' | 'SortAndCombine' | 'RemoveUnused';
}
interface ParserOptions {
organizeImportsSkipDestructiveCodeActions?: boolean;
organizeImportsMode?: 'All' | 'SortAndCombine' | 'RemoveUnused';
}
}

Expand Down
16 changes: 16 additions & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,22 @@ If you don't want destructive code actions (like removing unused imports), you c
}
```

### Mode
Use `organizeImportsMode` to determine how import organization works. This is the recommended approach, as TypeScript has deprecated the [skipDestructiveCodeActions](https://github.com/microsoft/TypeScript/blob/main/src/services/types.ts#L746C5-L746C31) option.

Available modes include:
- `All` - Performs complete import organization: sorting, combining, and removing unused imports
- `SortAndCombine` - Only sorts and combines imports without removing any
- `RemoveUnused` - Only removes unused imports without changing their order

The default mode is `All`. For compatibility, if `organizeImportsSkipDestructiveCodeActions` is set to `true`, the mode will be forced to `SortAndCombine`.

```json
{
"organizeImportsMode": 'All'
}
```

## Compatibility

### ESLint
Expand Down
37 changes: 37 additions & 0 deletions test/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,40 @@ test('does not remove unused imports with `organizeImportsSkipDestructiveCodeAct

t.is(formattedCode, code);
});

test('does not remove unused imports with `organizeImportsMode` set to `SortAndCombine`', async (t) => {
const code = `import { foo } from "./bar";
`;

const formattedCode = await prettify(code, { organizeImportsMode: 'SortAndCombine' });

t.is(formattedCode, code);
});

test('sort and combine with `organizeImportsMode` set to `SortAndCombine`', async (t) => {
const code = `
import { foo } from "foobar";
import { bar, baz } from "foobar";

const foobar = foo + baz
`;

const expect = 'import { bar, baz, foo } from "foobar";';

const formattedCode = await prettify(code, { organizeImportsMode: 'SortAndCombine' });

t.is(formattedCode.split('\n')[0], expect);
});

test('only remove unused imports with `organizeImportsMode` set to `RemoveUnused`', async (t) => {
const code = `
import { foo, bar, baz } from "foobar";

const foobar = foo + baz
`;
const expect = 'import { foo, baz } from "foobar";';

const formattedCode = await prettify(code, { organizeImportsMode: 'RemoveUnused' });

t.is(formattedCode.split('\n')[0], expect);
});