diff --git a/README.md b/README.md index 1da6656..f5456bf 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,7 @@ Currently, Steiger is not extendable with more rules, though that will change in segments-by-purpose Discourage the use of segment names that group code by its essence, and instead encourage grouping by purpose shared-lib-grouping Forbid having too many ungrouped modules in shared/lib. no-processes Discourage the use of the deprecated Processes layer. + import-locality Require that imports from the same slice be relative and imports from one slice to another be absolute. diff --git a/packages/steiger-plugin-fsd/src/import-locality/README.md b/packages/steiger-plugin-fsd/src/import-locality/README.md new file mode 100644 index 0000000..fe09b98 --- /dev/null +++ b/packages/steiger-plugin-fsd/src/import-locality/README.md @@ -0,0 +1,41 @@ +# `import-locality` + +Require that imports from the same slice be relative and imports from one slice to another be absolute. + +For example, `entities/user/ui/Avatar.tsx` should use relative imports when importing from `entities/user/ui/Avatar.styles.ts`, but use absolute imports when importing from `shared/ui`. + +Aliases are considered absolute imports, so setting up an alias is a nice way to do absolute imports. + +Example of a file whose imports pass this rule: + +```ts +// entities/user/ui/Avatar.tsx +import classes from './Avatar.styles' +import { shadows } from '@/shared/ui' +``` + +Examples of files whose imports violate this rule: + +```ts +// entities/user/ui/Avatar.tsx +import classes from '@/entities/user/ui/Avatar.styles.ts' +``` + +```ts +// entities/user/ui/Avatar.tsx +import classes from '@/entities/user' +``` + +```ts +// entities/user/ui/Avatar.tsx +import { shadows } from '../../../shared/ui' +``` + +## Rationale + +Imports between slices should be absolute to stay stable during refactors. If you change the folder structure inside your slice, imports from other slices should not change. + +Imports within a slice should not be absolute because having them absolute leads to one of the following cases: + +1. If the team wants to keep imports short (i.e. only `@/entities/user`), this leads to everything internal being exposed in the public API of the slice. +2. If the public API is kept to the necessary minimum, imports become unnecessarily longer because they include the layer, slice, and segment names. Usually the folder tree in a slice is not too deep, so relative imports will be short and also stable during slice renames. diff --git a/packages/steiger-plugin-fsd/src/import-locality/index.spec.ts b/packages/steiger-plugin-fsd/src/import-locality/index.spec.ts new file mode 100644 index 0000000..09f2b8b --- /dev/null +++ b/packages/steiger-plugin-fsd/src/import-locality/index.spec.ts @@ -0,0 +1,147 @@ +import { expect, it, vi } from 'vitest' + +import { parseIntoFsdRoot } from '../_lib/prepare-test.js' +import importLocality from './index.js' + +vi.mock('tsconfck', async (importOriginal) => { + return { + ...(await importOriginal()), + parse: vi.fn(() => Promise.resolve({ tsconfig: { compilerOptions: { paths: { '@/*': ['/*'] } } } })), + } +}) + +vi.mock('node:fs', async (importOriginal) => { + const originalFs = await importOriginal() + const { createFsMocks } = await import('../_lib/prepare-test.js') + + return createFsMocks( + { + '/shared/ui/styles.ts': '', + '/shared/ui/Button.tsx': 'import styles from "./styles";', + '/shared/ui/TextField.tsx': 'import styles from "./styles";', + '/shared/ui/index.ts': '', + '/entities/user/ui/Name.tsx': 'import { Button } from "@/shared/ui"', + '/entities/user/ui/Status.tsx': 'import { Button } from "@/shared/ui"; import { Name } from "@/entities/user"', + '/entities/user/ui/UserAvatar.tsx': + 'import { Button } from "@/shared/ui"; import { Name } from "@/entities/user/ui/Name"', + '/entities/user/index.ts': '', + '/features/comments/ui/CommentCard.tsx': 'import { Name } from "../../../entities/user"', + '/features/comments/index.ts': '', + '/pages/editor/ui/styles.ts': '', + '/pages/editor/ui/EditorPage.tsx': 'import { Button } from "@/shared/ui"; import { Editor } from "./Editor"', + '/pages/editor/ui/Editor.tsx': 'import { TextField } from "@/shared/ui"', + '/pages/editor/index.ts': '', + }, + originalFs, + ) +}) + +it('reports no errors on a project with relative imports within slices and absolute imports outside', async () => { + const root = parseIntoFsdRoot(` + 📂 shared + 📂 ui + 📄 styles.ts + 📄 Button.tsx + 📄 TextField.tsx + 📄 index.ts + 📂 pages + 📂 editor + 📂 ui + 📄 EditorPage.tsx + 📄 Editor.tsx + 📄 index.ts + `) + + expect((await importLocality.check(root)).diagnostics).toEqual([]) +}) + +it('reports errors on a project with absolute imports within a slice', async () => { + const root = parseIntoFsdRoot(` + 📂 shared + 📂 ui + 📄 styles.ts + 📄 Button.tsx + 📄 TextField.tsx + 📄 index.ts + 📂 entities + 📂 user + 📂 ui + 📄 Name.tsx + 📄 UserAvatar.tsx + 📄 index.ts + 📂 pages + 📂 editor + 📂 ui + 📄 EditorPage.tsx + 📄 Editor.tsx + 📄 index.ts + `) + + expect((await importLocality.check(root)).diagnostics).toEqual([ + { + message: 'Import on "entities/user" from "ui/UserAvatar.tsx" to "ui/Name.tsx" should be relative.', + }, + ]) +}) + +it('reports errors on a project with absolute imports from the index within a slice', async () => { + const root = parseIntoFsdRoot(` + 📂 shared + 📂 ui + 📄 styles.ts + 📄 Button.tsx + 📄 TextField.tsx + 📄 index.ts + 📂 entities + 📂 user + 📂 ui + 📄 Name.tsx + 📄 Status.tsx + 📄 index.ts + 📂 pages + 📂 editor + 📂 ui + 📄 EditorPage.tsx + 📄 Editor.tsx + 📄 index.ts + `) + + expect((await importLocality.check(root)).diagnostics).toEqual([ + { + message: 'Import on "entities/user" from "ui/Status.tsx" to "index.ts" should be relative.', + }, + ]) +}) + +it('reports errors on a project with relative imports between slices', async () => { + const root = parseIntoFsdRoot(` + 📂 shared + 📂 ui + 📄 styles.ts + 📄 Button.tsx + 📄 TextField.tsx + 📄 index.ts + 📂 entities + 📂 user + 📂 ui + 📄 Name.tsx + 📄 index.ts + 📂 features + 📂 comments + 📂 ui + 📄 CommentCard.tsx + 📄 index.ts + 📂 pages + 📂 editor + 📂 ui + 📄 EditorPage.tsx + 📄 Editor.tsx + 📄 index.ts + `) + + expect((await importLocality.check(root)).diagnostics).toEqual([ + { + message: 'Import from "features/comments/ui/CommentCard.tsx" to "entities/user/index.ts should not be relative.', + }, + ]) +}) diff --git a/packages/steiger-plugin-fsd/src/import-locality/index.ts b/packages/steiger-plugin-fsd/src/import-locality/index.ts index d7f9e77..47b2ab1 100644 --- a/packages/steiger-plugin-fsd/src/import-locality/index.ts +++ b/packages/steiger-plugin-fsd/src/import-locality/index.ts @@ -56,7 +56,7 @@ const importLocality = { const layerAndSlice = join(...[dependencyLocation.layerName, dependencyLocation.sliceName].filter(Boolean)) diagnostics.push({ - message: `Import from "${sourceRelativePath}" to "${dependencyRelativePath}" on "${layerAndSlice}" should be relative.`, + message: `Import on "${layerAndSlice}" from "${sourceRelativePath}" to "${dependencyRelativePath}" should be relative.`, }) } } diff --git a/packages/steiger/README.md b/packages/steiger/README.md index 1da6656..f5456bf 100644 --- a/packages/steiger/README.md +++ b/packages/steiger/README.md @@ -69,6 +69,7 @@ Currently, Steiger is not extendable with more rules, though that will change in segments-by-purpose Discourage the use of segment names that group code by its essence, and instead encourage grouping by purpose shared-lib-grouping Forbid having too many ungrouped modules in shared/lib. no-processes Discourage the use of the deprecated Processes layer. + import-locality Require that imports from the same slice be relative and imports from one slice to another be absolute.