Skip to content

Commit

Permalink
Add tests and docs for import-locality
Browse files Browse the repository at this point in the history
  • Loading branch information
illright committed Jun 18, 2024
1 parent 1805b11 commit 764186c
Show file tree
Hide file tree
Showing 5 changed files with 191 additions and 1 deletion.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ Currently, Steiger is not extendable with more rules, though that will change in
<tr> <td><a href="./packages/steiger-plugin-fsd/src/segments-by-purpose/README.md"><code>segments-by-purpose</code></a></td> <td>Discourage the use of segment names that group code by its essence, and instead encourage grouping by purpose</td> </tr>
<tr> <td><a href="./packages/steiger-plugin-fsd/src/shared-lib-grouping/README.md"><code>shared-lib-grouping</code></a></td> <td>Forbid having too many ungrouped modules in <code>shared/lib</code>.</td> </tr>
<tr> <td><a href="./packages/steiger-plugin-fsd/src/no-processes/README.md"><code>no-processes</code></a></td> <td>Discourage the use of the deprecated Processes layer.</td> </tr>
<tr> <td><a href="./packages/steiger-plugin-fsd/src/import-locality/README.md"><code>import-locality</code></a></td> <td>Require that imports from the same slice be relative and imports from one slice to another be absolute.</td> </tr>
</tbody>
</table>

Expand Down
41 changes: 41 additions & 0 deletions packages/steiger-plugin-fsd/src/import-locality/README.md
Original file line number Diff line number Diff line change
@@ -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.
147 changes: 147 additions & 0 deletions packages/steiger-plugin-fsd/src/import-locality/index.spec.ts
Original file line number Diff line number Diff line change
@@ -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<typeof import('tsconfck')>()),
parse: vi.fn(() => Promise.resolve({ tsconfig: { compilerOptions: { paths: { '@/*': ['/*'] } } } })),
}
})

vi.mock('node:fs', async (importOriginal) => {
const originalFs = await importOriginal<typeof import('fs')>()
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.',
},
])
})
2 changes: 1 addition & 1 deletion packages/steiger-plugin-fsd/src/import-locality/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.`,
})
}
}
Expand Down
1 change: 1 addition & 0 deletions packages/steiger/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ Currently, Steiger is not extendable with more rules, though that will change in
<tr> <td><a href="./packages/steiger-plugin-fsd/src/segments-by-purpose/README.md"><code>segments-by-purpose</code></a></td> <td>Discourage the use of segment names that group code by its essence, and instead encourage grouping by purpose</td> </tr>
<tr> <td><a href="./packages/steiger-plugin-fsd/src/shared-lib-grouping/README.md"><code>shared-lib-grouping</code></a></td> <td>Forbid having too many ungrouped modules in <code>shared/lib</code>.</td> </tr>
<tr> <td><a href="./packages/steiger-plugin-fsd/src/no-processes/README.md"><code>no-processes</code></a></td> <td>Discourage the use of the deprecated Processes layer.</td> </tr>
<tr> <td><a href="./packages/steiger-plugin-fsd/src/import-locality/README.md"><code>import-locality</code></a></td> <td>Require that imports from the same slice be relative and imports from one slice to another be absolute.</td> </tr>
</tbody>
</table>

Expand Down

0 comments on commit 764186c

Please sign in to comment.