Skip to content

Commit

Permalink
[New] order: add sortTypesGroup option to allow intragroup sortin…
Browse files Browse the repository at this point in the history
…g of type-only imports

Closes import-js#2912
Closes import-js#2347
Closes import-js#2441
Subsumes import-js#2615
  • Loading branch information
Xunnamius authored and ljharb committed Dec 23, 2024
1 parent 341178d commit fd1ae63
Show file tree
Hide file tree
Showing 4 changed files with 274 additions and 7 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
- add [`enforce-node-protocol-usage`] rule and `import/node-version` setting ([#3024], thanks [@GoldStrikeArch] and [@sevenc-nanashi])
- add TypeScript types ([#3097], thanks [@G-Rath])
- [`extensions`]: add `pathGroupOverrides to allow enforcement decision overrides based on specifier ([#3105], thanks [@Xunnamius])
- [`order`]: add `sortTypesGroup` option to allow intragroup sorting of type-only imports ([#3104], thanks [@Xunnamius])

### Fixed
- [`no-unused-modules`]: provide more meaningful error message when no .eslintrc is present ([#3116], thanks [@michaelfaith])
Expand Down Expand Up @@ -1174,6 +1175,7 @@ for info on changes for earlier releases.
[#3116]: https://github.com/import-js/eslint-plugin-import/pull/3116
[#3106]: https://github.com/import-js/eslint-plugin-import/pull/3106
[#3105]: https://github.com/import-js/eslint-plugin-import/pull/3105
[#3104]: https://github.com/import-js/eslint-plugin-import/pull/3104
[#3097]: https://github.com/import-js/eslint-plugin-import/pull/3097
[#3073]: https://github.com/import-js/eslint-plugin-import/pull/3073
[#3072]: https://github.com/import-js/eslint-plugin-import/pull/3072
Expand Down
64 changes: 62 additions & 2 deletions docs/rules/order.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ This rule supports the following options (none of which are required):
- [`alphabetize`][30]
- [`named`][33]
- [`warnOnUnassignedImports`][5]
- [`sortTypesGroup`][7]

---

Expand Down Expand Up @@ -156,7 +157,7 @@ Roughly speaking, the grouping algorithm is as follows:

1. If the import has no corresponding identifiers (e.g. `import './my/thing.js'`), is otherwise "unassigned," or is an unsupported use of `require()`, and [`warnOnUnassignedImports`][5] is disabled, it will be ignored entirely since the order of these imports may be important for their [side-effects][31]
2. If the import is part of an arcane TypeScript declaration (e.g. `import log = console.log`), it will be considered **object**. However, note that external module references (e.g. `import x = require('z')`) are treated as normal `require()`s and import-exports (e.g. `export import w = y;`) are ignored entirely
3. If the import is [type-only][6], and `"type"` is in `groups`, it will be considered **type** (with additional implications if using [`pathGroups`][8] and `"type"` is in [`pathGroupsExcludedImportTypes`][9])
3. If the import is [type-only][6], `"type"` is in `groups`, and [`sortTypesGroup`][7] is disabled, it will be considered **type** (with additional implications if using [`pathGroups`][8] and `"type"` is in [`pathGroupsExcludedImportTypes`][9])
4. If the import's specifier matches [`import/internal-regex`][28], it will be considered **internal**
5. If the import's specifier is an absolute path, it will be considered **unknown**
6. If the import's specifier has the name of a Node.js core module (using [is-core-module][10]), it will be considered **builtin**
Expand All @@ -171,7 +172,7 @@ Roughly speaking, the grouping algorithm is as follows:
15. If the import's specifier has a name that starts with a word character, it will be considered **external**
16. If this point is reached, the import will be ignored entirely

At the end of the process, if they co-exist in the same file, all top-level `require()` statements that haven't been ignored are shifted (with respect to their order) below any ES6 `import` or similar declarations.
At the end of the process, if they co-exist in the same file, all top-level `require()` statements that haven't been ignored are shifted (with respect to their order) below any ES6 `import` or similar declarations. Finally, any type-only declarations are potentially reorganized according to [`sortTypesGroup`][7].

### `pathGroups`

Expand Down Expand Up @@ -533,6 +534,64 @@ import path from 'path';
import './styles.css';
```

### `sortTypesGroup`

Valid values: `boolean` \
Default: `false`

> \[!NOTE]
>
> This setting is only meaningful when `"type"` is included in [`groups`][18].
Sort [type-only imports][6] separately from normal non-type imports.

When enabled, the intragroup sort order of [type-only imports][6] will mirror the intergroup ordering of normal imports as defined by [`groups`][18], [`pathGroups`][8], etc.

#### Example

Given the following settings:

```jsonc
{
"import/order": ["error", {
"groups": ["type", "builtin", "parent", "sibling", "index"],
"alphabetize": { "order": "asc" }
}]
}
```

This will fail the rule check even though it's logically ordered as we expect (builtins come before parents, parents come before siblings, siblings come before indices), the only difference is we separated type-only imports from normal imports:

```ts
import type A from "fs";
import type B from "path";
import type C from "../foo.js";
import type D from "./bar.js";
import type E from './';

import a from "fs";
import b from "path";
import c from "../foo.js";
import d from "./bar.js";
import e from "./";
```

This happens because [type-only imports][6] are considered part of one global
[`"type"` group](#how-imports-are-grouped) by default. However, if we set
`sortTypesGroup` to `true`:

```jsonc
{
"import/order": ["error", {
"groups": ["type", "builtin", "parent", "sibling", "index"],
"alphabetize": { "order": "asc" },
"sortTypesGroup": true
}]
}
```

The same example will pass.

## Related

- [`import/external-module-folders`][29]
Expand All @@ -543,6 +602,7 @@ import './styles.css';
[4]: https://nodejs.org/api/esm.html#terminology
[5]: #warnonunassignedimports
[6]: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-8.html#type-only-imports-and-export
[7]: #sorttypesgroup
[8]: #pathgroups
[9]: #pathgroupsexcludedimporttypes
[10]: https://www.npmjs.com/package/is-core-module
Expand Down
33 changes: 28 additions & 5 deletions src/rules/order.js
Original file line number Diff line number Diff line change
Expand Up @@ -513,31 +513,43 @@ function computePathRank(ranks, pathGroups, path, maxPosition) {
}
}

function computeRank(context, ranks, importEntry, excludedImportTypes) {
function computeRank(context, ranks, importEntry, excludedImportTypes, isSortingTypesGroup) {
let impType;
let rank;

const isTypeGroupInGroups = ranks.omittedTypes.indexOf('type') === -1;
const isTypeOnlyImport = importEntry.node.importKind === 'type';
const isExcludedFromPathRank = isTypeOnlyImport && isTypeGroupInGroups && excludedImportTypes.has('type');

if (importEntry.type === 'import:object') {
impType = 'object';
} else if (importEntry.node.importKind === 'type' && ranks.omittedTypes.indexOf('type') === -1) {
} else if (isTypeOnlyImport && isTypeGroupInGroups && !isSortingTypesGroup) {
impType = 'type';
} else {
impType = importType(importEntry.value, context);
}
if (!excludedImportTypes.has(impType)) {

if (!excludedImportTypes.has(impType) && !isExcludedFromPathRank) {
rank = computePathRank(ranks.groups, ranks.pathGroups, importEntry.value, ranks.maxPosition);
}

if (typeof rank === 'undefined') {
rank = ranks.groups[impType];
}

if (isTypeOnlyImport && isSortingTypesGroup) {
rank = ranks.groups.type + rank / 10;
}

if (importEntry.type !== 'import' && !importEntry.type.startsWith('import:')) {
rank += 100;
}

return rank;
}

function registerNode(context, importEntry, ranks, imported, excludedImportTypes) {
const rank = computeRank(context, ranks, importEntry, excludedImportTypes);
function registerNode(context, importEntry, ranks, imported, excludedImportTypes, isSortingTypesGroup) {
const rank = computeRank(context, ranks, importEntry, excludedImportTypes, isSortingTypesGroup);
if (rank !== -1) {
imported.push({ ...importEntry, rank });
}
Expand Down Expand Up @@ -781,6 +793,10 @@ module.exports = {
'never',
],
},
sortTypesGroup: {
type: 'boolean',
default: false,
},
named: {
default: false,
oneOf: [{
Expand Down Expand Up @@ -837,6 +853,7 @@ module.exports = {
const options = context.options[0] || {};
const newlinesBetweenImports = options['newlines-between'] || 'ignore';
const pathGroupsExcludedImportTypes = new Set(options.pathGroupsExcludedImportTypes || ['builtin', 'external', 'object']);
const sortTypesGroup = options.sortTypesGroup;

const named = {
types: 'mixed',
Expand Down Expand Up @@ -879,6 +896,9 @@ module.exports = {
const importMap = new Map();
const exportMap = new Map();

const isTypeGroupInGroups = ranks.omittedTypes.indexOf('type') === -1;
const isSortingTypesGroup = isTypeGroupInGroups && sortTypesGroup;

function getBlockImports(node) {
if (!importMap.has(node)) {
importMap.set(node, []);
Expand Down Expand Up @@ -932,6 +952,7 @@ module.exports = {
ranks,
getBlockImports(node.parent),
pathGroupsExcludedImportTypes,
isSortingTypesGroup,
);

if (named.import) {
Expand Down Expand Up @@ -983,6 +1004,7 @@ module.exports = {
ranks,
getBlockImports(node.parent),
pathGroupsExcludedImportTypes,
isSortingTypesGroup,
);
},
CallExpression(node) {
Expand All @@ -1005,6 +1027,7 @@ module.exports = {
ranks,
getBlockImports(block),
pathGroupsExcludedImportTypes,
isSortingTypesGroup,
);
},
...named.require && {
Expand Down
Loading

0 comments on commit fd1ae63

Please sign in to comment.