Skip to content

Commit

Permalink
Fix type/non-type sorting (#47)
Browse files Browse the repository at this point in the history
We had a bug, identified in #35 (comment), which was causing the `"^[./]"` group to include both type and value imports, instead of only value imports.

This PR fixes that bug, and expands both the unit and e2e tests, which fail without this change and pass with it.
  • Loading branch information
IanVS committed Oct 27, 2022
1 parent 92d0028 commit 6f5cbbc
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 6 deletions.
34 changes: 34 additions & 0 deletions src/utils/__tests__/get-import-nodes-matched-group.spec.ts
Expand Up @@ -12,6 +12,16 @@ import j from './j';
import l from './l';
import a from '@core/a';
`;

const codeWithTypes = `
import z from 'z';
import type T from 't';
import {b} from 'b';
import a from './a';
import type {F} from 'f';
import type Local from './local';
`;

test('should return correct matched groups', () => {
const importNodes = getImportNodes(code);
const importOrder = [
Expand Down Expand Up @@ -53,3 +63,27 @@ test('should return THIRD_PARTY_MODULES as matched group with empty order list',
expect(matchedGroup).toEqual('<THIRD_PARTY_MODULES>');
}
});

test('should sort non-types separately from types if <TYPES> group is present', () => {
const importNodes = getImportNodes(codeWithTypes, {
plugins: ['typescript'],
});
const importOrder = ['^[./]', '<TYPES>^[./]', '<TYPES>'];

let matchedGroups: string[] = [];
for (const importNode of importNodes) {
const matchedGroup = getImportNodesMatchedGroup(
importNode,
importOrder,
);
matchedGroups.push(matchedGroup);
}
expect(matchedGroups).toEqual([
'<THIRD_PARTY_MODULES>',
'<TYPES>',
'<THIRD_PARTY_MODULES>',
'^[./]',
'<TYPES>',
'<TYPES>^[./]',
]);
});
9 changes: 8 additions & 1 deletion src/utils/get-import-nodes-matched-group.ts
Expand Up @@ -7,6 +7,9 @@ import {

/**
* Get the regexp group to keep the import nodes.
*
* This comes near the end of processing, after import declaration nodes have been combined or exploded.
*
* @param node
* @param importOrder
*/
Expand Down Expand Up @@ -36,7 +39,11 @@ export const getImportNodesMatchedGroup = (
node.importKind === 'type' &&
node.source.value.match(regExp) !== null;
} else {
matched = node.source.value.match(regExp) !== null;
// If <TYPES> is being used for any group, and this group doesn't have it, only look for value imports
matched = includesTypesSpecialWord
? node.importKind !== 'type' &&
node.source.value.match(regExp) !== null
: node.source.value.match(regExp) !== null;
}

if (matched) return group;
Expand Down
10 changes: 6 additions & 4 deletions tests/TypesSpecialWord/__snapshots__/ppsi.spec.js.snap
Expand Up @@ -2,17 +2,19 @@

exports[`imports-with-mixed-declarations.ts - typescript-verify: imports-with-mixed-declarations.ts 1`] = `
import a, {type LocalType} from './local-file';
import type Another from './another-local-file';
import value, {tp} from 'third-party';
import {specifier, type ThirdPartyType} from 'third-party';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
import type { ThirdPartyType } from "third-party";
import value, { tp, specifier } from "third-party";
import type { LocalType } from "./local-file";
import a from "./local-file";
import type Another from "./another-local-file";
import type { LocalType } from "./local-file";
`;

exports[`imports-with-third-party-types.ts - typescript-verify: imports-with-third-party-types.ts 1`] = `
Expand All @@ -29,9 +31,9 @@ import type { SpecifierType } from "third-party";
import value, { specifier } from "third-party";
import a from "./local-file";
import type LocalType from "./local-file";
import type { LocalSpecifierType } from "./local-file";
import a from "./local-file";
`;
1 change: 1 addition & 0 deletions tests/TypesSpecialWord/imports-with-mixed-declarations.ts
@@ -1,3 +1,4 @@
import a, {type LocalType} from './local-file';
import type Another from './another-local-file';
import value, {tp} from 'third-party';
import {specifier, type ThirdPartyType} from 'third-party';
2 changes: 1 addition & 1 deletion tests/TypesSpecialWord/ppsi.spec.js
Expand Up @@ -2,8 +2,8 @@ run_spec(__dirname, ['typescript'], {
importOrder: [
'<TYPES>',
'<THIRD_PARTY_MODULES>',
'<TYPES>^[./]',
'^[./]',
'<TYPES>^[./]',
],
importOrderSeparation: true,
importOrderMergeDuplicateImports: true,
Expand Down

0 comments on commit 6f5cbbc

Please sign in to comment.