Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add no-segments-on-sliced-layers rule #54

Merged
6 changes: 6 additions & 0 deletions .changeset/mean-pianos-cover.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@feature-sliced/steiger-plugin': minor
'steiger': minor
---

Add no-segments-on-sliced-layers rule
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ Currently, Steiger is not extendable with more rules, though that will change in
<tr> <td><a href="./packages/steiger-plugin-fsd/src/no-public-api-sidestep/README.md"><code>no-public-api-sidestep</code></a></td> <td>Forbid going around the public API of a slice to import directly from an internal module in a slice.</td> </tr>
<tr> <td><a href="./packages/steiger-plugin-fsd/src/no-reserved-folder-names/README.md"><code>no-reserved-folder-names</code></a></td> <td>Forbid subfolders in segments that have the same name as other conventional segments.</td> </tr>
<tr> <td><a href="./packages/steiger-plugin-fsd/src/no-segmentless-slices/README.md"><code>no-segmentless-slices</code></a></td> <td>Forbid slices that don't have any segments.</td> </tr>
<tr> <td><a href="./packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/README.md"><code>no-segments-on-sliced-layers</code></a></td> <td>Forbid segments (like ui, lib, api ...) that appear directly in sliced layer folders (entities, features, ...)</td> </tr>
<tr> <td><a href="./packages/steiger-plugin-fsd/src/public-api/README.md"><code>public-api</code></a></td> <td>Require slices (and segments on sliceless layers like Shared) to have a public API definition.</td> </tr>
<tr> <td><a href="./packages/steiger-plugin-fsd/src/repetitive-naming/README.md"><code>repetitive-naming</code></a></td> <td>Ensure that all entities are named consistently in terms of pluralization.</td> </tr>
<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>
Expand Down
2 changes: 2 additions & 0 deletions packages/steiger-plugin-fsd/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import noLayerPublicApi from './no-layer-public-api/index.js'
import noPublicApiSidestep from './no-public-api-sidestep/index.js'
import noReservedFolderNames from './no-reserved-folder-names/index.js'
import noSegmentlessSlices from './no-segmentless-slices/index.js'
import noSegmentsOnSlicedLayers from './no-segments-on-sliced-layers/index.js'
import publicApi from './public-api/index.js'
import repetitiveNaming from './repetitive-naming/index.js'
import segmentsByPurpose from './segments-by-purpose/index.js'
Expand All @@ -23,6 +24,7 @@ export default [
noPublicApiSidestep,
noReservedFolderNames,
noSegmentlessSlices,
noSegmentsOnSlicedLayers,
publicApi,
repetitiveNaming,
segmentsByPurpose,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# `no-segmentless-slices`

Forbid segments that appear in direct children of sliced layers.

Examples of project structures that pass this rule:

```
📂 shared
📂 ui
📄 index.ts
📂 lib
📄 index.ts
📂 entities
📂 user
📂 ui
📂 model
📄 index.ts
📂 pages
📂 home
📂 ui
📄 index.ts
```

Examples of project structures that fail this rule:

```
📂 shared
📂 ui
📄 index.ts
📂 lib
📄 index.ts
📂 entities
📂 user
📂 ui
📂 model
📄 index.ts
📂 api // ❌
📄 index.ts
📂 pages
📂 home
📂 ui
📄 index.ts
```

## Rationale

Several folder names like `ui` or `api` are conventionally understood in FSD as segments. When you have segments as direct children, it's either because that's the name you chose for your slice, or because some code ended up unsliced. In the first case, where that is the name of your slice, such a name is likely to cause confusion among developers who are familiar with FSD. In the second case, where a segment on a layer is because there was no slice to put it in, it's a violation of FSD structure, which decreases the benefits of FSD and also causes confusion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { describe, expect, it } from 'vitest'

import noSegmentsOnSlicedLayers from './index.js'
import { joinFromRoot, parseIntoFsdRoot, compareMessages } from '../_lib/prepare-test.js'

describe('no-segments-on-sliced-layers rule', () => {
it('reports no errors on a project where the sliced layers has no segments in direct children', () => {
const root = parseIntoFsdRoot(`
📂 shared
📂 ui
📄 index.ts
📂 i18n
📄 index.ts
📂 entities
📂 user
📂 ui
📄 Name.tsx
📂 api
📄 useCurrentUser.ts
📄 index.ts
📂 document
📂 api
📄 useDocument.ts
📂 pages
📂 home
📂 ui
📄 index.ts
`)

expect(noSegmentsOnSlicedLayers.check(root)).toEqual({ diagnostics: [] })
})

it('reports errors on a project where a sliced layer has segments among its direct children', () => {
const root = parseIntoFsdRoot(`
📂 shared
📂 ui
📄 index.ts
📂 i18n
📄 index.ts
📂 entities
📂 user
📄 index.ts
📄 Name.tsx
📂 ui
📄 index.ts
📂 features
📂 user
📂 ui
📄 LogIn.tsx
📄 index.ts
📄 index.ts
📂 api
📄 index.ts
📂 widgets
📂 footer
📂 ui
📄 Footer.tsx
📄 index.ts
📄 index.ts
📂 config
📄 index.ts
📂 pages
📂 home
📂 ui
📄 index.ts
📂 settings
📂 profile
📄 ProfilePage.tsx
📄 index.ts
📂 lib
📄 index.ts
`)

const diagnostics = noSegmentsOnSlicedLayers.check(root).diagnostics.toSorted(compareMessages)

expect(diagnostics).toEqual([
{
message:
'Conventional segment "api" should not be a direct child of a sliced layer. Consider moving it inside a slice or, if that is a slice, consider a different name for it to avoid confusion with segments.',
location: { path: joinFromRoot('features', 'api') },
},
{
message:
'Conventional segment "config" should not be a direct child of a sliced layer. Consider moving it inside a slice or, if that is a slice, consider a different name for it to avoid confusion with segments.',
location: { path: joinFromRoot('widgets', 'config') },
},
{
message:
'Conventional segment "lib" should not be a direct child of a sliced layer. Consider moving it inside a slice or, if that is a slice, consider a different name for it to avoid confusion with segments.',
location: { path: joinFromRoot('pages', 'lib') },
},
{
message:
'Conventional segment "ui" should not be a direct child of a sliced layer. Consider moving it inside a slice or, if that is a slice, consider a different name for it to avoid confusion with segments.',
location: { path: joinFromRoot('entities', 'ui') },
},
])
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { basename } from 'node:path'

import { getLayers, isSliced, conventionalSegmentNames } from '@feature-sliced/filesystem'
import type { Diagnostic, Rule } from '@steiger/types'

const noSegmentsOnSlicedLayers = {
name: 'no-segments-on-sliced-layers',
check(root) {
const diagnostics: Array<Diagnostic> = []
const layers = Object.values(getLayers(root))

for (const layer of layers) {
if (isSliced(layer)) {
for (const directChild of layer.children) {
if (directChild.type === 'folder') {
const folderName = basename(directChild.path)
const isConventionalSegment = folderName && conventionalSegmentNames.includes(folderName)

if (isConventionalSegment) {
diagnostics.push({
message: `Conventional segment "${folderName}" should not be a direct child of a sliced layer. Consider moving it inside a slice or, if that is a slice, consider a different name for it to avoid confusion with segments.`,
location: { path: directChild.path },
})
}
}
}
}
}

return { diagnostics }
},
} satisfies Rule

export default noSegmentsOnSlicedLayers
1 change: 1 addition & 0 deletions packages/steiger/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ Currently, Steiger is not extendable with more rules, though that will change in
<tr> <td><a href="./packages/steiger-plugin-fsd/src/no-public-api-sidestep/README.md"><code>no-public-api-sidestep</code></a></td> <td>Forbid going around the public API of a slice to import directly from an internal module in a slice.</td> </tr>
<tr> <td><a href="./packages/steiger-plugin-fsd/src/no-reserved-folder-names/README.md"><code>no-reserved-folder-names</code></a></td> <td>Forbid subfolders in segments that have the same name as other conventional segments.</td> </tr>
<tr> <td><a href="./packages/steiger-plugin-fsd/src/no-segmentless-slices/README.md"><code>no-segmentless-slices</code></a></td> <td>Forbid slices that don't have any segments.</td> </tr>
<tr> <td><a href="./packages/steiger-plugin-fsd/src/no-segments-on-sliced-layers/README.md"><code>no-segments-on-sliced-layers</code></a></td> <td>Forbid segments (like ui, lib, api ...) that appear directly in sliced layer folders (entities, features, ...)</td> </tr>
<tr> <td><a href="./packages/steiger-plugin-fsd/src/public-api/README.md"><code>public-api</code></a></td> <td>Require slices (and segments on sliceless layers like Shared) to have a public API definition.</td> </tr>
<tr> <td><a href="./packages/steiger-plugin-fsd/src/repetitive-naming/README.md"><code>repetitive-naming</code></a></td> <td>Ensure that all entities are named consistently in terms of pluralization.</td> </tr>
<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>
Expand Down
1 change: 1 addition & 0 deletions packages/steiger/src/models/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export const schema = z.object({
'no-public-api-sidestep',
'no-reserved-folder-names',
'no-segmentless-slices',
'no-segments-on-sliced-layers',
'public-api',
'repetitive-naming',
'segments-by-purpose',
Expand Down
Loading