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

Replace importOrderCombineTypeAndValueImports with importOrderTypeScriptVersion #67

Merged
merged 5 commits into from
Apr 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
30 changes: 7 additions & 23 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Since then more critical features & fixes have been added, and the options have

- Do not re-order across side-effect imports
- Combine imports from the same source
- Combine type and value imports ([`importOrderCombineTypeAndValueImports`](#importordercombinetypeandvalueimports))
- Combine type and value imports
- Type import grouping with `<TYPES>` keyword
- Sorts node.js builtin modules to top
- Custom import order separation
Expand All @@ -31,7 +31,7 @@ Since then more critical features & fixes have been added, and the options have
- [How does import sort work?](#how-does-import-sort-work)
- [Options](#options)
- [`importOrder`](#importorder)
- [`importOrderCombineTypeAndValueImports`](#importordercombinetypeandvalueimports)
- [`importOrderTypeScriptVersion`](#importordertypescriptversion)
- [`importOrderParserPlugins`](#importorderparserplugins)
- [Prevent imports from being sorted](#prevent-imports-from-being-sorted)
- [FAQ / Troubleshooting](#faq--troubleshooting)
Expand Down Expand Up @@ -129,12 +129,10 @@ module.exports = {
semi: true,
importOrder: ['^@core/(.*)$', '', '^@server/(.*)$', '', '^@ui/(.*)$', '', '^[./]'],
importOrderParserPlugins: ['typescript', 'jsx', 'decorators-legacy'],
importOrderCombineTypeAndValueImports: true,
importOrderTypeScriptVersion: '5.0.0',
};
```

_Note: all flags are off by default, so explore your options [below](#options)_

### How does import sort work?

The plugin extracts the imports which are defined in `importOrder`. These imports are considered as _local imports_.
Expand Down Expand Up @@ -217,27 +215,13 @@ _Note:_ If you want to separate some groups from others, you can add an empty st
],
```

#### `importOrderCombineTypeAndValueImports`

**type**: `boolean`

**default value:** `false`
#### `importOrderTypeScriptVersion`

A boolean value to control merging `import type` expressions into `import {…}`.
**type**: `string`

```diff
- import type { C1 } from 'c';
- import { C2 } from 'c';
+ import { type C1, C2 } from "c";
**default value:** `1.0.0`

- import { D1 } from 'd';
- import type { D2 } from 'd';
+ import { D1, type D2 } from "d";

- import type { A1 } from 'a';
- import type { A2 } from 'a';
+ import type { A1, A2 } from "a";
```
When using TypeScript, some import syntax can only be used in newer versions of TypeScript. If you would like to enable modern features like mixed type and value imports, set this option to the semver version string of the TypeScript in use in your project.

#### `importOrderParserPlugins`

Expand Down
12 changes: 12 additions & 0 deletions docs/MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
- The `importOrderCaseInsensitive` option has been removed, and imports will always be sorted case-insensitive.
- The `importOrderGroupNamespaceSpecifiers` option has been removed.
- The `importOrderSortSpecifiers` option has been removed, and specifiers are now always sorted (previous `true` setting)
- The `importOrderMergeDuplicateImports` option has been removed, and imports are always combined (previous `true` setting)
- The `importOrderCombineTypeAndValueImports` option has been removed. See [below](#importOrderCombineTypeAndValueImports-removed) for details
- Added `importOrderTypeScriptVersion` option.

#### `importOrderSeparation` removed

Expand Down Expand Up @@ -38,6 +41,15 @@ Or, if you would like to keep all imports together, but add a newline before sid
]
```

#### `importOrderCombineTypeAndValueImports` removed

Combining type and value imports is supported in Flow and TypeScript 4.5.0 and above. To simplify the configuration of the plugin, the explicit setting has been removed. Instead, we will always enable combining these imports when using Flow and have introduced a new option, `importOrderTypeScriptVersion` to control whether or not merging can happen when using TypeScript.

#### `importOrderTypeScriptVersion` added

Some import statement syntax can only be used in certain versions of TypeScript. In order to enable these features, such as merging type and value imports, you can specify the version of TypeScript that you're using in your project using this option, which should be a valid semver string.


### Migrating from v2.x.x to v3.x.x

#### TL;DR
Expand Down
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@
"@babel/traverse": "^7.17.3",
"@babel/types": "^7.17.0",
"lodash.clone": "^4.5.0",
"lodash.isequal": "^4.5.0"
"lodash.isequal": "^4.5.0",
"semver": "^7.5.0"
},
"devDependencies": {
"@types/babel__generator": "^7.6.4",
Expand All @@ -67,6 +68,7 @@
"@types/lodash.isequal": "4.5.6",
"@types/node": "^18.15.13",
"@types/prettier": "^2.7.2",
"@types/semver": "^7.3.13",
"@vue/compiler-sfc": "3.2.47",
"prettier": "2.8.7",
"typescript": "5.0.4",
Expand Down
1 change: 1 addition & 0 deletions prettier.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ module.exports = {
semi: true,
plugins: [require('./lib/src/index.js')],
importOrder: ['', '<THIRD_PARTY_MODULES>', '', '^[./]'],
importOrderTypeScriptVersion: '5.0.0',
};
2 changes: 1 addition & 1 deletion src/constants.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { builtinModules } from 'module';

import { ParserPlugin } from '@babel/parser';
import type { ParserPlugin } from '@babel/parser';
import { expressionStatement, stringLiteral } from '@babel/types';

export const flow: ParserPlugin = 'flow';
Expand Down
8 changes: 4 additions & 4 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@ export const options: Record<
default: [{ value: ['typescript', 'jsx'] }],
description: 'Provide a list of plugins for special syntax',
},
importOrderCombineTypeAndValueImports: {
type: 'boolean',
importOrderTypeScriptVersion: {
type: 'string',
category: 'Global',
default: false,
default: '1.0.0',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we default to 1.0.0?

This is a breaking change, so we could default to 5.0.0 instead

Copy link
Owner Author

@IanVS IanVS Apr 25, 2023

Choose a reason for hiding this comment

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

My reasoning is in #22 (comment), but happy to hear any counter arguments. Especially the first point. I think choosing 5.0 would address the second.

Copy link
Collaborator

Choose a reason for hiding this comment

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

#22 (comment)

We could alternately not default unless we can't detect :P

Copy link
Owner Author

Choose a reason for hiding this comment

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

I responded. I think that there are risks we'll detect the wrong version, but I think I'm okay with that as long as we keep this option as an override / escape hatch in case the auto-detection is incorrect.

What do you think about running with just the explicit version for right now, and adding in auto-detection in another PR?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No objections.

I just wanted to advocate for the thing that helps people that are starting new projects! That's likely the first spike of interest in a tool like this, followed by new people joining mature (or stagnant) complex codebases.

I think that if people that are stuck on old versions, that's not a surprise to them, and they will notice the "1 other option" that tells them to pass their TypeScript version if they have a complex setup!

I also think it's unlikely that typescript will drastically change import syntax in the future -- it has to stay in sync with the wider JS Community there.

Anyhow, feel free to proceed without auto-detection now. Technically that can always be added in a future release. Technically a breaking behavior change, but… I wouldn't follow SemVer when changing that -- just my 2 cents.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thanks for the excellent discussion!

description:
'Should import-type expressions be merged into import-value expressions?',
'Version of TypeScript in use in the project. Determines some output syntax when using TypeScript.',
},
};

Expand Down
29 changes: 21 additions & 8 deletions src/preprocessors/preprocessor.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { parse as babelParser, ParserOptions } from '@babel/parser';
import traverse, { NodePath } from '@babel/traverse';
import { ImportDeclaration, isTSModuleDeclaration } from '@babel/types';
import semver from 'semver';

import { TYPES_SPECIAL_WORD } from '../constants';
import { PrettierOptions } from '../types';
Expand All @@ -10,25 +11,37 @@ import { getSortedNodes } from '../utils/get-sorted-nodes';

export function preprocessor(code: string, options: PrettierOptions): string {
const { importOrderParserPlugins, importOrder } = options;
let { importOrderTypeScriptVersion } = options;
const isTSSemverValid = semver.valid(importOrderTypeScriptVersion);

let { importOrderCombineTypeAndValueImports } = options;

if (
importOrderCombineTypeAndValueImports &&
importOrder.some((group) => group.includes(TYPES_SPECIAL_WORD))
) {
if (!isTSSemverValid) {
console.warn(
`[@ianvs/prettier-plugin-sort-imports]: The option importOrderCombineTypeAndValueImports will have no effect since ${TYPES_SPECIAL_WORD} is used in importOrder.`,
`[@ianvs/prettier-plugin-sort-imports]: The option importOrderTypeScriptVersion is not a valid semver version and will be ignored.`,
);
importOrderCombineTypeAndValueImports = false;
importOrderTypeScriptVersion = '1.0.0';
}

// Do not combine type and value imports if `<TYPES>` is specified explicitly
let importOrderCombineTypeAndValueImports = importOrder.some((group) =>
group.includes(TYPES_SPECIAL_WORD),
)
? false
: true;

const allOriginalImportNodes: ImportDeclaration[] = [];
const parserOptions: ParserOptions = {
sourceType: 'module',
plugins: getExperimentalParserPlugins(importOrderParserPlugins),
};

// Disable importOrderCombineTypeAndValueImports if typescript is not set to a version that supports it
if (
parserOptions.plugins?.includes('typescript') &&
semver.lt(importOrderTypeScriptVersion, '4.5.0')
) {
importOrderCombineTypeAndValueImports = false;
}

const ast = babelParser(code, parserOptions);

const directives = ast.program.directives;
Expand Down
7 changes: 3 additions & 4 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,9 @@ export type ImportOrLine = ImportDeclaration | ExpressionStatement;

export type GetSortedNodes = (
nodes: ImportDeclaration[],
options: Pick<
PrettierOptions,
'importOrder' | 'importOrderCombineTypeAndValueImports'
>,
options: Pick<PrettierOptions, 'importOrder'> & {
importOrderCombineTypeAndValueImports: boolean;
},
) => ImportOrLine[];

export type GetChunkTypeOfNode = (node: ImportDeclaration) => ChunkType;
Expand Down
10 changes: 7 additions & 3 deletions src/utils/__tests__/get-all-comments-from-nodes.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { ParserOptions } from '@babel/parser';
import { CommentBlock, CommentLine, ImportDeclaration } from '@babel/types';
import type { ParserOptions } from '@babel/parser';
import type {
CommentBlock,
CommentLine,
ImportDeclaration,
} from '@babel/types';
import { expect, test } from 'vitest';

import { getAllCommentsFromNodes } from '../get-all-comments-from-nodes';
Expand All @@ -11,7 +15,7 @@ const getSortedImportNodes = (code: string, options?: ParserOptions) => {

return getSortedNodes(importNodes, {
importOrder: [],
importOrderCombineTypeAndValueImports: false,
importOrderCombineTypeAndValueImports: true,
});
};

Expand Down
7 changes: 3 additions & 4 deletions src/utils/__tests__/get-code-from-ast.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import a from 'a';
const importNodes = getImportNodes(code);
const sortedNodes = getSortedNodes(importNodes, {
importOrder: [],
importOrderCombineTypeAndValueImports: false,
importOrderCombineTypeAndValueImports: true,
});
const formatted = getCodeFromAst({
nodesToOutput: sortedNodes,
Expand Down Expand Up @@ -54,7 +54,7 @@ import type {See} from 'c';
const importNodes = getImportNodes(code, { plugins: ['typescript'] });
const sortedNodes = getSortedNodes(importNodes, {
importOrder: [],
importOrderCombineTypeAndValueImports: false,
importOrderCombineTypeAndValueImports: true,
});
const formatted = getCodeFromAst({
nodesToOutput: sortedNodes,
Expand All @@ -66,8 +66,7 @@ import type {See} from 'c';
`// first comment
// second comment
import a, { b, type Bee } from "a";
import c from "c";
import type { C, See } from "c";
import c, { type C, type See } from "c";
import g from "g";
import k from "k";
import t from "t";
Expand Down
16 changes: 8 additions & 8 deletions src/utils/__tests__/get-sorted-nodes-by-import-order.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ImportDeclaration } from '@babel/types';
import type { ImportDeclaration } from '@babel/types';
import { expect, test } from 'vitest';

import { getImportNodes } from '../get-import-nodes';
Expand Down Expand Up @@ -29,7 +29,7 @@ test('it returns all sorted nodes', () => {
const result = getImportNodes(code);
const sorted = getSortedNodesByImportOrder(result, {
importOrder: ['^[./]'],
importOrderCombineTypeAndValueImports: false,
importOrderCombineTypeAndValueImports: true,
}) as ImportDeclaration[];

expect(getSortedNodesNamesAndNewlines(sorted)).toEqual([
Expand Down Expand Up @@ -78,7 +78,7 @@ test('it returns all sorted nodes with sort order', () => {
const result = getImportNodes(code);
const sorted = getSortedNodesByImportOrder(result, {
importOrder: ['^a$', '^t$', '^k$', '^B', '^[./]'],
importOrderCombineTypeAndValueImports: false,
importOrderCombineTypeAndValueImports: true,
}) as ImportDeclaration[];
expect(getSortedNodesNamesAndNewlines(sorted)).toEqual([
'node:fs/promises',
Expand Down Expand Up @@ -134,7 +134,7 @@ import {type B, A} from 'z';
});
const sorted = getSortedNodesByImportOrder(result, {
importOrder: ['^[./]'],
importOrderCombineTypeAndValueImports: false,
importOrderCombineTypeAndValueImports: true,
}) as ImportDeclaration[];
expect(getSortedNodesNamesAndNewlines(sorted)).toEqual(['k', 't', 'z']);
expect(
Expand All @@ -154,7 +154,7 @@ test('it returns all sorted nodes with builtin specifiers at the top', () => {
const result = getImportNodes(code);
const sorted = getSortedNodesByImportOrder(result, {
importOrder: ['^[./]'],
importOrderCombineTypeAndValueImports: false,
importOrderCombineTypeAndValueImports: true,
}) as ImportDeclaration[];

expect(getSortedNodesNamesAndNewlines(sorted)).toEqual([
Expand All @@ -180,7 +180,7 @@ test('it returns all sorted nodes with custom third party modules and builtins a
const result = getImportNodes(code);
const sorted = getSortedNodesByImportOrder(result, {
importOrder: ['^a$', '<THIRD_PARTY_MODULES>', '^t$', '^k$', '^[./]'],
importOrderCombineTypeAndValueImports: false,
importOrderCombineTypeAndValueImports: true,
}) as ImportDeclaration[];
expect(getSortedNodesNamesAndNewlines(sorted)).toEqual([
'node:fs/promises',
Expand Down Expand Up @@ -212,7 +212,7 @@ test('it returns all sorted nodes with custom separation', () => {
'^k$',
'^[./]',
],
importOrderCombineTypeAndValueImports: false,
importOrderCombineTypeAndValueImports: true,
}) as ImportDeclaration[];
expect(getSortedNodesNamesAndNewlines(sorted)).toEqual([
'node:fs/promises',
Expand Down Expand Up @@ -247,7 +247,7 @@ test('it does not add multiple custom import separators', () => {
'^k$',
'^[./]',
],
importOrderCombineTypeAndValueImports: false,
importOrderCombineTypeAndValueImports: true,
}) as ImportDeclaration[];
expect(getSortedNodesNamesAndNewlines(sorted)).toEqual([
'node:fs/promises',
Expand Down
4 changes: 2 additions & 2 deletions src/utils/__tests__/get-sorted-nodes.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ImportDeclaration } from '@babel/types';
import type { ImportDeclaration } from '@babel/types';
import { expect, test } from 'vitest';

import { getImportNodes } from '../get-import-nodes';
Expand Down Expand Up @@ -30,7 +30,7 @@ test('it returns all sorted nodes, preserving the order side effect nodes', () =
const result = getImportNodes(code);
const sorted = getSortedNodes(result, {
importOrder: [],
importOrderCombineTypeAndValueImports: false,
importOrderCombineTypeAndValueImports: true,
}) as ImportDeclaration[];
expect(getSortedNodesNamesAndNewlines(sorted)).toEqual([
'se3',
Expand Down
Loading