Skip to content

Commit

Permalink
Fix type import ordering (#331)
Browse files Browse the repository at this point in the history
* fix type import ordering

* Add a snapshot test for the bugfix
  • Loading branch information
rsslldnphy authored Dec 9, 2024
1 parent b769c7c commit 668adfa
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 14 deletions.
63 changes: 63 additions & 0 deletions src/utils/__tests__/get-import-nodes-matched-group.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,69 @@ test('should return correct matched groups', () => {
]);
});

test('should return type imports as part of a matching group if no type-specific group is present', () => {
const code = `
import type { ExternalType } from 'external-type-module';
import type { InternalType } from './internal-type-module';
import { externalFn } from 'external-fn-module';
import { internalFn } from './internal-fn-module';
`
const importNodes = getImportNodes(code,{
plugins: ['typescript'],
});
const importOrder = [
'^[^.].*',
'^[.].*',
];

let matchedGroups: string[] = [];
for (const importNode of importNodes) {
const matchedGroup = getImportNodesMatchedGroup(
importNode,
importOrder,
);
matchedGroups.push(matchedGroup);
}
expect(matchedGroups).toEqual([
'^[^.].*',
'^[.].*',
'^[^.].*',
'^[.].*',
]);
});

test('should return type imports as part of a type-specific group even if a matching non-type specific group precedes it', () => {
const code = `
import type { ExternalType } from 'external-type-module';
import type { InternalType } from './internal-type-module';
import { externalFn } from 'external-fn-module';
import { internalFn } from './internal-fn-module';
`
const importNodes = getImportNodes(code, {
plugins: ['typescript'],
});
const importOrder = [
'^[^.].*',
'^[.].*',
'<TS_TYPES>^[.].*',
];

let matchedGroups: string[] = [];
for (const importNode of importNodes) {
const matchedGroup = getImportNodesMatchedGroup(
importNode,
importOrder,
);
matchedGroups.push(matchedGroup);
}
expect(matchedGroups).toEqual([
'^[^.].*',
'<TS_TYPES>^[.].*',
'^[^.].*',
'^[.].*',
]);
});

test('should return THIRD_PARTY_MODULES as matched group with empty order list', () => {
const importNodes = getImportNodes(code);
const importOrder: string[] = [];
Expand Down
42 changes: 28 additions & 14 deletions src/utils/get-import-nodes-matched-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,35 @@ export const getImportNodesMatchedGroup = (
: new RegExp(group),
}));

for (const { group, regExp } of groupWithRegExp) {
// finding the group for non-type imports is easy: it's the first group that matches.
// however, for type imports, we need to make sure that we don't match a non-type group
// that's earlier in the list than a type-specific group that would otherwise match.
// so we need to get all matching groups, look for the first matching _type-specific_ group,
// and if it exists, return it. otherwise, return the first matching group if there is one.
const matchingGroups = groupWithRegExp.filter(({ group, regExp }) => {
if (
(group.startsWith(TYPES_SPECIAL_WORD) &&
node.importKind !== 'type') ||
(!group.startsWith(TYPES_SPECIAL_WORD) &&
node.importKind === 'type')
)
continue;
group.startsWith(TYPES_SPECIAL_WORD) &&
node.importKind !== 'type'
) {
return false;
} else {
return node.source.value.match(regExp) !== null;
}
});

const matched = node.source.value.match(regExp) !== null;
if (matched) return group;
if (matchingGroups.length === 0) {
return node.importKind === 'type' &&
importOrder.find(
(group) => group === THIRD_PARTY_TYPES_SPECIAL_WORD,
)
? THIRD_PARTY_TYPES_SPECIAL_WORD
: THIRD_PARTY_MODULES_SPECIAL_WORD;
} else if (node.importKind !== 'type') {
return matchingGroups[0].group;
} else {
for (const { group } of matchingGroups) {
if (group.startsWith(TYPES_SPECIAL_WORD)) return group;
}
return matchingGroups[0].group;
}

return node.importKind === 'type' &&
importOrder.find((group) => group === THIRD_PARTY_TYPES_SPECIAL_WORD)
? THIRD_PARTY_TYPES_SPECIAL_WORD
: THIRD_PARTY_MODULES_SPECIAL_WORD;
};
19 changes: 19 additions & 0 deletions tests/ImportsSeparated/__snapshots__/ppsi.spec.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,25 @@ export const a = 1;
`;

exports[`import-with-type-imports-together.ts - typescript-verify: import-with-type-imports-together.ts 1`] = `
import { foo } from "@server/foo"
import type { Quux } from "./quux"
import { Link } from "@ui/Link"
import type { Bar } from "@server/bar"
import type { LinkProps } from "@ui/Link/LinkProps"
import { baz } from "./baz"
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
import type { Bar } from "@server/bar";
import { foo } from "@server/foo";
import { Link } from "@ui/Link";
import type { LinkProps } from "@ui/Link/LinkProps";
import { baz } from "./baz";
import type { Quux } from "./quux";
`;

exports[`imports-with-comments.ts - typescript-verify: imports-with-comments.ts 1`] = `
// I am top level comment in this file.
// I am second line of top level comment in this file.
Expand Down
6 changes: 6 additions & 0 deletions tests/ImportsSeparated/import-with-type-imports-together.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { foo } from "@server/foo"
import type { Quux } from "./quux"
import { Link } from "@ui/Link"
import type { Bar } from "@server/bar"
import type { LinkProps } from "@ui/Link/LinkProps"
import { baz } from "./baz"

0 comments on commit 668adfa

Please sign in to comment.