Skip to content

Commit

Permalink
Remove importOrderMergeDuplicateImports option (always on) (#66)
Browse files Browse the repository at this point in the history
Ref: #22 

This removes the `importOrderMergeDuplicateImports` option. I can't
think of a reason you'd need to have multiple imports from the same
module in different statements, and it can cause confusion if you do
have them.
  • Loading branch information
IanVS committed Apr 24, 2023
1 parent a41e794 commit c16fdd7
Show file tree
Hide file tree
Showing 15 changed files with 7 additions and 155 deletions.
12 changes: 1 addition & 11 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ Since then more critical features & fixes have been added, and the options have
**Features not currently supported by upstream:**

- Do not re-order across side-effect imports
- Combine imports from the same source ([`importOrderMergeDuplicateImports`](#importordermergeduplicateimports))
- Combine imports from the same source
- Combine type and value imports ([`importOrderCombineTypeAndValueImports`](#importordercombinetypeandvalueimports))
- Type import grouping with `<TYPES>` keyword
- Sorts node.js builtin modules to top
Expand All @@ -31,7 +31,6 @@ 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)
- [`importOrderMergeDuplicateImports`](#importordermergeduplicateimports)
- [`importOrderCombineTypeAndValueImports`](#importordercombinetypeandvalueimports)
- [`importOrderParserPlugins`](#importorderparserplugins)
- [Prevent imports from being sorted](#prevent-imports-from-being-sorted)
Expand Down Expand Up @@ -130,7 +129,6 @@ module.exports = {
semi: true,
importOrder: ['^@core/(.*)$', '', '^@server/(.*)$', '', '^@ui/(.*)$', '', '^[./]'],
importOrderParserPlugins: ['typescript', 'jsx', 'decorators-legacy'],
importOrderMergeDuplicateImports: true,
importOrderCombineTypeAndValueImports: true,
};
```
Expand Down Expand Up @@ -219,14 +217,6 @@ _Note:_ If you want to separate some groups from others, you can add an empty st
],
```

#### `importOrderMergeDuplicateImports`

**type**: `boolean`

**default value:** `false`

When `true`, multiple import statements from the same module will be combined into a single import.

#### `importOrderCombineTypeAndValueImports`

**type**: `boolean`
Expand Down
6 changes: 0 additions & 6 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,6 @@ export const options: Record<
default: [{ value: ['typescript', 'jsx'] }],
description: 'Provide a list of plugins for special syntax',
},
importOrderMergeDuplicateImports: {
type: 'boolean',
category: 'Global',
default: false,
description: 'Should duplicate imports be merged?',
},
importOrderCombineTypeAndValueImports: {
type: 'boolean',
category: 'Global',
Expand Down
16 changes: 1 addition & 15 deletions src/preprocessors/preprocessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,10 @@ import { getExperimentalParserPlugins } from '../utils/get-experimental-parser-p
import { getSortedNodes } from '../utils/get-sorted-nodes';

export function preprocessor(code: string, options: PrettierOptions): string {
const {
importOrderParserPlugins,
importOrder,
importOrderMergeDuplicateImports,
} = options;
const { importOrderParserPlugins, importOrder } = options;

let { importOrderCombineTypeAndValueImports } = options;

if (
importOrderCombineTypeAndValueImports &&
!importOrderMergeDuplicateImports
) {
console.warn(
'[@ianvs/prettier-plugin-sort-imports]: The option importOrderCombineTypeAndValueImports will have no effect since importOrderMergeDuplicateImports is not also enabled.',
);
}

if (
importOrderCombineTypeAndValueImports &&
importOrder.some((group) => group.includes(TYPES_SPECIAL_WORD))
Expand Down Expand Up @@ -65,7 +52,6 @@ export function preprocessor(code: string, options: PrettierOptions): string {

const nodesToOutput = getSortedNodes(allOriginalImportNodes, {
importOrder,
importOrderMergeDuplicateImports,
importOrderCombineTypeAndValueImports,
});

Expand Down
4 changes: 1 addition & 3 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,7 @@ export type GetSortedNodes = (
nodes: ImportDeclaration[],
options: Pick<
PrettierOptions,
| 'importOrder'
| 'importOrderMergeDuplicateImports'
| 'importOrderCombineTypeAndValueImports'
'importOrder' | 'importOrderCombineTypeAndValueImports'
>,
) => ImportOrLine[];

Expand Down
1 change: 0 additions & 1 deletion src/utils/__tests__/get-all-comments-from-nodes.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ const getSortedImportNodes = (code: string, options?: ParserOptions) => {

return getSortedNodes(importNodes, {
importOrder: [],
importOrderMergeDuplicateImports: false,
importOrderCombineTypeAndValueImports: false,
});
};
Expand Down
2 changes: 0 additions & 2 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,6 @@ import a from 'a';
const importNodes = getImportNodes(code);
const sortedNodes = getSortedNodes(importNodes, {
importOrder: [],
importOrderMergeDuplicateImports: false,
importOrderCombineTypeAndValueImports: false,
});
const formatted = getCodeFromAst({
Expand Down Expand Up @@ -55,7 +54,6 @@ import type {See} from 'c';
const importNodes = getImportNodes(code, { plugins: ['typescript'] });
const sortedNodes = getSortedNodes(importNodes, {
importOrder: [],
importOrderMergeDuplicateImports: true,
importOrderCombineTypeAndValueImports: false,
});
const formatted = getCodeFromAst({
Expand Down
7 changes: 0 additions & 7 deletions src/utils/__tests__/get-sorted-nodes-by-import-order.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ test('it returns all sorted nodes', () => {
const result = getImportNodes(code);
const sorted = getSortedNodesByImportOrder(result, {
importOrder: ['^[./]'],
importOrderMergeDuplicateImports: false,
importOrderCombineTypeAndValueImports: false,
}) as ImportDeclaration[];

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

Expand All @@ -184,7 +180,6 @@ 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$', '^[./]'],
importOrderMergeDuplicateImports: false,
importOrderCombineTypeAndValueImports: false,
}) as ImportDeclaration[];
expect(getSortedNodesNamesAndNewlines(sorted)).toEqual([
Expand Down Expand Up @@ -217,7 +212,6 @@ test('it returns all sorted nodes with custom separation', () => {
'^k$',
'^[./]',
],
importOrderMergeDuplicateImports: false,
importOrderCombineTypeAndValueImports: false,
}) as ImportDeclaration[];
expect(getSortedNodesNamesAndNewlines(sorted)).toEqual([
Expand Down Expand Up @@ -253,7 +247,6 @@ test('it does not add multiple custom import separators', () => {
'^k$',
'^[./]',
],
importOrderMergeDuplicateImports: false,
importOrderCombineTypeAndValueImports: false,
}) as ImportDeclaration[];
expect(getSortedNodesNamesAndNewlines(sorted)).toEqual([
Expand Down
1 change: 0 additions & 1 deletion src/utils/__tests__/get-sorted-nodes.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ test('it returns all sorted nodes, preserving the order side effect nodes', () =
const result = getImportNodes(code);
const sorted = getSortedNodes(result, {
importOrder: [],
importOrderMergeDuplicateImports: false,
importOrderCombineTypeAndValueImports: false,
}) as ImportDeclaration[];
expect(getSortedNodesNamesAndNewlines(sorted)).toEqual([
Expand Down
88 changes: 0 additions & 88 deletions src/utils/__tests__/merge-nodes-with-matching-flavors.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { getSortedNodes } from '../get-sorted-nodes';

const defaultOptions = {
importOrder: [''], // Separate side-effect and ignored chunks, for easier test readability
importOrderMergeDuplicateImports: false,
importOrderCombineTypeAndValueImports: false,
};

Expand Down Expand Up @@ -46,7 +45,6 @@ test('should merge duplicate imports within a given chunk', () => {

const nodesToOutput = getSortedNodes(allOriginalImportNodes, {
...defaultOptions,
importOrderMergeDuplicateImports: true,
importOrderCombineTypeAndValueImports: false,
});
const formatted = getCodeFromAst({
Expand Down Expand Up @@ -106,7 +104,6 @@ test('should merge type imports into regular imports', () => {

const nodesToOutput = getSortedNodes(allOriginalImportNodes, {
...defaultOptions,
importOrderMergeDuplicateImports: true,
importOrderCombineTypeAndValueImports: true,
});
const formatted = getCodeFromAst({
Expand Down Expand Up @@ -139,7 +136,6 @@ import defaultValue from './source';

const nodesToOutput = getSortedNodes(allOriginalImportNodes, {
...defaultOptions,
importOrderMergeDuplicateImports: true,
importOrderCombineTypeAndValueImports: true,
});
const formatted = getCodeFromAst({
Expand All @@ -165,7 +161,6 @@ import * as Namespace from './source';

const nodesToOutput = getSortedNodes(allOriginalImportNodes, {
...defaultOptions,
importOrderMergeDuplicateImports: true,
importOrderCombineTypeAndValueImports: true,
});
const formatted = getCodeFromAst({
Expand All @@ -192,7 +187,6 @@ import {value as alias} from './source';

const nodesToOutput = getSortedNodes(allOriginalImportNodes, {
...defaultOptions,
importOrderMergeDuplicateImports: true,
importOrderCombineTypeAndValueImports: true,
});
const formatted = getCodeFromAst({
Expand All @@ -218,7 +212,6 @@ import {value, SecondValue} from './source';

const nodesToOutput = getSortedNodes(allOriginalImportNodes, {
...defaultOptions,
importOrderMergeDuplicateImports: true,
importOrderCombineTypeAndValueImports: true,
});
const formatted = getCodeFromAst({
Expand Down Expand Up @@ -246,7 +239,6 @@ import {otherValue} from './other';

const nodesToOutput = getSortedNodes(allOriginalImportNodes, {
...defaultOptions,
importOrderMergeDuplicateImports: true,
importOrderCombineTypeAndValueImports: true,
});
const formatted = getCodeFromAst({
Expand Down Expand Up @@ -275,7 +267,6 @@ import {SecondValue} from './source';

const nodesToOutput = getSortedNodes(allOriginalImportNodes, {
...defaultOptions,
importOrderMergeDuplicateImports: true,
importOrderCombineTypeAndValueImports: true,
});
const formatted = getCodeFromAst({
Expand Down Expand Up @@ -303,7 +294,6 @@ import {value} from './source';

const nodesToOutput = getSortedNodes(allOriginalImportNodes, {
...defaultOptions,
importOrderMergeDuplicateImports: true,
importOrderCombineTypeAndValueImports: true,
});
const formatted = getCodeFromAst({
Expand All @@ -320,83 +310,6 @@ import { thirdValue } from "./third";
`);
});

test("doesn't merge duplicate imports if option disabled", () => {
const code = `
import type { A } from 'a';
import { Junk } from 'junk-group-1'
import type { B } from 'a';
import "./side-effects1";
// C, E and D will be separated from A, B because side-effects in-between
import type { C } from 'a';
import { D } from "a";
import type { E } from "a";
// prettier-ignore
import type { NoMerge1 } from "a";
// prettier-ignore
import { NoMerge2 } from "a";
import { H } from 'b';
import { F } from 'a';
// F Will be alone because prettier-ignore in-between
import { G } from 'b';
import * as J from 'c';
import { Junk2 } from 'junk-group-2'
import * as K from "c";
// * as J, * as K can't merge because both Namespaces
import {I} from "c"
import { default as Def2 } from 'd';
import { default as Def1 } from 'd';
import Foo1 from 'e';
import Foo2 from 'e';
`;
const allOriginalImportNodes = getImportNodes(code, {
plugins: ['typescript'],
});

const nodesToOutput = getSortedNodes(
allOriginalImportNodes,
defaultOptions,
);
const formatted = getCodeFromAst({
nodesToOutput,
allOriginalImportNodes,
originalCode: code,
directives: [],
});

expect(format(formatted, { parser: 'babel' }))
.toEqual(`import type { A } from "a";
import type { B } from "a";
import { Junk } from "junk-group-1";
import "./side-effects1";
// C, E and D will be separated from A, B because side-effects in-between
import type { C } from "a";
import { D } from "a";
import type { E } from "a";
// prettier-ignore
import type { NoMerge1 } from "a";
// prettier-ignore
import { NoMerge2 } from "a";
import { F } from "a";
import { H } from "b";
// F Will be alone because prettier-ignore in-between
import { G } from "b";
import * as J from "c";
import * as K from "c";
// * as J, * as K can't merge because both Namespaces
import { I } from "c";
import { default as Def2 } from "d";
import { default as Def1 } from "d";
import Foo1 from "e";
import Foo2 from "e";
import { Junk2 } from "junk-group-2";
`);
});

test('should not combine default type imports', () => {
const code = `
import { ComponentProps, useEffect } from "react";
Expand All @@ -408,7 +321,6 @@ test('should not combine default type imports', () => {

const nodesToOutput = getSortedNodes(allOriginalImportNodes, {
...defaultOptions,
importOrderMergeDuplicateImports: true,
importOrderCombineTypeAndValueImports: true,
});
const formatted = getCodeFromAst({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ test('it should remove nodes from the original code', () => {
const importNodes = getImportNodes(code);
const sortedNodes = getSortedNodes(importNodes, {
importOrder: [],
importOrderMergeDuplicateImports: false,
importOrderCombineTypeAndValueImports: false,
});
const allCommentsFromImports = getAllCommentsFromNodes(sortedNodes);
Expand Down
14 changes: 4 additions & 10 deletions src/utils/get-sorted-nodes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,7 @@ import { mergeNodesWithMatchingImportFlavors } from './merge-nodes-with-matching
* @returns A sorted array of the remaining import nodes
*/
export const getSortedNodes: GetSortedNodes = (nodes, options) => {
const {
importOrder,
importOrderMergeDuplicateImports,
importOrderCombineTypeAndValueImports,
} = options;
const { importOrder, importOrderCombineTypeAndValueImports } = options;

// Split nodes at each boundary between a side-effect node and a
// non-side-effect node, keeping both types of nodes together.
Expand Down Expand Up @@ -68,11 +64,9 @@ export const getSortedNodes: GetSortedNodes = (nodes, options) => {
finalNodes.push(...chunk.nodes);
}
} else {
let nodes = importOrderMergeDuplicateImports
? mergeNodesWithMatchingImportFlavors(chunk.nodes, {
importOrderCombineTypeAndValueImports,
})
: chunk.nodes;
let nodes = mergeNodesWithMatchingImportFlavors(chunk.nodes, {
importOrderCombineTypeAndValueImports,
});
// If type ordering is specified explicitly, we need to break apart type and value specifiers
if (
importOrder.some((group) => group.includes(TYPES_SPECIAL_WORD))
Expand Down
1 change: 0 additions & 1 deletion tests/Flow/ppsi.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,5 @@ import {run_spec} from '../../test-setup/run_spec';
run_spec(__dirname, ['flow'], {
importOrder: ['^@core/(.*)$', '^@server/(.*)', '^@ui/(.*)$', '^[./]'],
importOrderParserPlugins: ['flow'],
importOrderMergeDuplicateImports: true,
importOrderCombineTypeAndValueImports: true,
});
Loading

0 comments on commit c16fdd7

Please sign in to comment.