-
Notifications
You must be signed in to change notification settings - Fork 18
feat(steiger-plugin): add no-wildcard-exports rule and fix tsconfig resolution #215
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,61 @@ | ||||||
# `no-wildcard-exports` | ||||||
|
||||||
Forbid wildcard exports (`export *`) in public APIs of business logic layers. Named exports and namespace exports (`export * as namespace`) are allowed. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: I'm a bit confused by the "business logic layers" here — I would consider suggestion: perhaps you meant "all layers except
Suggested change
|
||||||
|
||||||
**Exception:** Wildcard exports are allowed in unsliced layers (`shared` and `app`), as they serve as foundational layers with different architectural purposes. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: I would actually argue against allowing wildcard exports even in Shared and App. To me, wildcard exports conceal the public API of a group of modules, which really hurts the ability to discover a codebase shallowly, i. e. without digging into every file. question: why do you think they should be allowed there? And do you agree with the content of the Public API reference page in the FSD docs? |
||||||
|
||||||
This rule treats files named `index.js`, `index.jsx`, `index.ts`, `index.tsx` as the public API of a folder (slice/segment root). Non-index files (including test files like `*.spec.ts`, `*.test.ts`) are ignored. | ||||||
|
||||||
Examples of exports that pass this rule: | ||||||
|
||||||
```ts | ||||||
// Named exports (business logic layers) | ||||||
export { Button } from './Button' | ||||||
export { UserCard, UserAvatar } from './components' | ||||||
|
||||||
// Namespace exports (all layers) | ||||||
export * as userModel from './model' | ||||||
export * as positions from './tooltip-positions' | ||||||
|
||||||
// Wildcard exports (unsliced layers: shared, app) | ||||||
// shared/ui/index.ts | ||||||
export * from './Button' | ||||||
export * from './Modal' | ||||||
export * from './Input' | ||||||
|
||||||
// shared/api/index.ts | ||||||
export * from './endpoints/auth' | ||||||
export * from './endpoints/users' | ||||||
|
||||||
// app/providers/index.ts | ||||||
export * from './AuthProvider' | ||||||
export * from './ThemeProvider' | ||||||
export * from './RouterProvider' | ||||||
``` | ||||||
|
||||||
Examples of exports that fail this rule: | ||||||
|
||||||
```ts | ||||||
// ❌ Wildcard exports in business logic layers | ||||||
// entities/user/index.ts | ||||||
export * from './model' | ||||||
export * from './ui' | ||||||
|
||||||
// features/auth/index.ts | ||||||
export * from './ui' | ||||||
export * from './api' | ||||||
``` | ||||||
|
||||||
## Rationale | ||||||
|
||||||
Wildcard exports in business logic layers make it harder to track which exact entities are being exported from a module. This can lead to: | ||||||
|
||||||
- Unintentionally exposing internal implementation details | ||||||
- Difficulty in tracking dependencies between modules | ||||||
- Potential naming conflicts when multiple modules use wildcard exports | ||||||
Comment on lines
+53
to
+55
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: I would also add the point about hurting shallow discovery (if you agree with it, that is :D) |
||||||
|
||||||
Using named exports or namespace exports makes the public API more explicit and easier to maintain in business logic layers. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: let's also leave a link to the docs about Public API |
||||||
|
||||||
## Autofix | ||||||
|
||||||
This rule provides a suggested fix for wildcard exports in public API files by replacing them with a named export template (e.g. `export { ComponentA, ComponentB } from './components'`). |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,258 @@ | ||
import { expect, it } from 'vitest' | ||
import { joinFromRoot, parseIntoFolder as parseIntoFsdRoot } from '@steiger/toolkit/test' | ||
import type { Folder, File } from '@steiger/toolkit' | ||
|
||
import noWildcardExports from './index.js' | ||
|
||
type FileWithContent = File & { content?: string } | ||
|
||
it('reports no errors on a project with valid exports', () => { | ||
const root = parseIntoFsdRoot(` | ||
📂 shared | ||
📂 ui | ||
📄 Button.tsx | ||
📄 index.ts | ||
📂 entities | ||
📂 user | ||
📂 ui | ||
📄 UserCard.tsx | ||
📄 index.ts | ||
📄 index.ts | ||
📂 features | ||
📂 auth | ||
📂 ui | ||
📄 LoginForm.tsx | ||
📄 index.ts | ||
`) | ||
|
||
function addContentToFiles(folder: Folder): void { | ||
for (const child of folder.children) { | ||
if (child.type === 'file') { | ||
const fileWithContent = child as FileWithContent | ||
if (child.path === joinFromRoot('shared', 'ui', 'index.ts')) { | ||
fileWithContent.content = "export { Button } from './Button'" | ||
} else if (child.path === joinFromRoot('entities', 'user', 'ui', 'index.ts')) { | ||
fileWithContent.content = "export { UserCard } from './UserCard'" | ||
} else if (child.path === joinFromRoot('entities', 'user', 'index.ts')) { | ||
fileWithContent.content = "export * as userModel from './model'" | ||
} else if (child.path === joinFromRoot('features', 'auth', 'ui', 'index.ts')) { | ||
fileWithContent.content = "export { LoginForm } from './LoginForm'" | ||
} else { | ||
fileWithContent.content = '' | ||
} | ||
} else if (child.type === 'folder') { | ||
addContentToFiles(child) | ||
} | ||
} | ||
} | ||
|
||
addContentToFiles(root) | ||
|
||
expect(noWildcardExports.check(root)).toEqual({ diagnostics: [] }) | ||
}) | ||
|
||
it('reports errors on a project with wildcard exports', () => { | ||
const root = parseIntoFsdRoot(` | ||
📂 shared | ||
📂 ui | ||
📄 Button.tsx | ||
📄 index.ts | ||
📂 entities | ||
📂 user | ||
📂 ui | ||
📄 UserCard.tsx | ||
📄 index.ts | ||
📄 index.ts | ||
`) | ||
|
||
function addContentToFiles(folder: Folder): void { | ||
for (const child of folder.children) { | ||
if (child.type === 'file') { | ||
const fileWithContent = child as FileWithContent | ||
if (child.path === joinFromRoot('shared', 'ui', 'index.ts')) { | ||
fileWithContent.content = "export * from './Button'" | ||
} else if (child.path === joinFromRoot('entities', 'user', 'ui', 'index.ts')) { | ||
fileWithContent.content = "export * from './UserCard'" | ||
} else { | ||
fileWithContent.content = '' | ||
} | ||
} else if (child.type === 'folder') { | ||
addContentToFiles(child) | ||
} | ||
} | ||
} | ||
|
||
addContentToFiles(root) | ||
|
||
const diagnostics = noWildcardExports.check(root).diagnostics | ||
expect(diagnostics).toEqual([ | ||
{ | ||
message: 'Wildcard exports are not allowed in public APIs. Use named exports instead.', | ||
location: { path: joinFromRoot('entities', 'user', 'ui', 'index.ts') }, | ||
fixes: [ | ||
{ | ||
type: 'modify-file', | ||
path: joinFromRoot('entities', 'user', 'ui', 'index.ts'), | ||
content: '// Replace with named exports\n// Example: export { ComponentA, ComponentB } from "./components"', | ||
}, | ||
], | ||
}, | ||
]) | ||
}) | ||
|
||
it('allows export * as namespace pattern', () => { | ||
const root = parseIntoFsdRoot(` | ||
📂 shared | ||
📂 ui | ||
📄 positions.ts | ||
📄 index.ts | ||
📂 entities | ||
📂 user | ||
📄 model.ts | ||
📄 index.ts | ||
`) | ||
|
||
function addContentToFiles(folder: Folder): void { | ||
for (const child of folder.children) { | ||
if (child.type === 'file') { | ||
const fileWithContent = child as FileWithContent | ||
if (child.path === joinFromRoot('shared', 'ui', 'index.ts')) { | ||
fileWithContent.content = "export * as positions from './positions'" | ||
} else if (child.path === joinFromRoot('entities', 'user', 'index.ts')) { | ||
fileWithContent.content = "export * as userModel from './model'" | ||
} else { | ||
fileWithContent.content = '' | ||
} | ||
} else if (child.type === 'folder') { | ||
addContentToFiles(child) | ||
} | ||
} | ||
} | ||
Comment on lines
+115
to
+130
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: I think this conceals the fact that this test will actually not pass during real usage, since the linter engine doesn't include the suggestion: let's use FS mocking for tests, like it's done in the test file for |
||
|
||
addContentToFiles(root) | ||
|
||
expect(noWildcardExports.check(root)).toEqual({ diagnostics: [] }) | ||
}) | ||
|
||
it('ignores wildcard exports in non-public files', () => { | ||
const root = parseIntoFsdRoot(` | ||
📂 shared | ||
📂 ui | ||
📄 internal.ts | ||
📄 index.ts | ||
📂 entities | ||
📂 user | ||
📂 ui | ||
📄 internal-utils.ts | ||
📄 index.ts | ||
`) | ||
|
||
function addContentToFiles(folder: Folder): void { | ||
for (const child of folder.children) { | ||
if (child.type === 'file') { | ||
const fileWithContent = child as FileWithContent | ||
if (child.path.endsWith('internal.ts')) { | ||
fileWithContent.content = "export * from './components'" | ||
} else if (child.path.endsWith('internal-utils.ts')) { | ||
fileWithContent.content = "export * from './utils'" | ||
} else { | ||
fileWithContent.content = '' | ||
} | ||
} else if (child.type === 'folder') { | ||
addContentToFiles(child) | ||
} | ||
} | ||
} | ||
|
||
addContentToFiles(root) | ||
|
||
expect(noWildcardExports.check(root)).toEqual({ diagnostics: [] }) | ||
}) | ||
|
||
it('ignores wildcard exports in test files', () => { | ||
const root = parseIntoFsdRoot(` | ||
📂 shared | ||
📂 ui | ||
📄 Button.test.ts | ||
📄 index.ts | ||
📂 entities | ||
📂 user | ||
📂 ui | ||
📄 UserCard.spec.ts | ||
📄 index.ts | ||
`) | ||
|
||
function addContentToFiles(folder: Folder): void { | ||
for (const child of folder.children) { | ||
if (child.type === 'file') { | ||
const fileWithContent = child as FileWithContent | ||
if (child.path.endsWith('Button.test.ts')) { | ||
fileWithContent.content = "export * from './test-utils'" | ||
} else if (child.path.endsWith('UserCard.spec.ts')) { | ||
fileWithContent.content = "export * from './test-utils'" | ||
} else { | ||
fileWithContent.content = '' | ||
} | ||
} else if (child.type === 'folder') { | ||
addContentToFiles(child) | ||
} | ||
} | ||
} | ||
|
||
addContentToFiles(root) | ||
|
||
expect(noWildcardExports.check(root)).toEqual({ diagnostics: [] }) | ||
}) | ||
|
||
it('allows wildcard exports in unsliced layers (shared and app)', () => { | ||
const root = parseIntoFsdRoot(` | ||
📂 shared | ||
📂 ui | ||
📄 Button.tsx | ||
📄 Modal.tsx | ||
📄 index.ts | ||
📂 api | ||
📄 client.ts | ||
📄 endpoints.ts | ||
📄 index.ts | ||
📂 app | ||
📂 providers | ||
📄 AuthProvider.tsx | ||
📄 ThemeProvider.tsx | ||
📄 index.ts | ||
📂 routes | ||
📄 index.ts | ||
📂 entities | ||
📂 user | ||
📂 ui | ||
📄 UserCard.tsx | ||
📄 index.ts | ||
`) | ||
|
||
function addContentToFiles(folder: Folder): void { | ||
for (const child of folder.children) { | ||
if (child.type === 'file') { | ||
const fileWithContent = child as FileWithContent | ||
if (child.path === joinFromRoot('shared', 'ui', 'index.ts')) { | ||
fileWithContent.content = "export * from './Button'\nexport * from './Modal'" | ||
} else if (child.path === joinFromRoot('shared', 'api', 'index.ts')) { | ||
fileWithContent.content = "export * from './client'\nexport * from './endpoints'" | ||
} else if (child.path === joinFromRoot('app', 'providers', 'index.ts')) { | ||
fileWithContent.content = "export * from './AuthProvider'\nexport * from './ThemeProvider'" | ||
} else if (child.path === joinFromRoot('app', 'routes', 'index.ts')) { | ||
fileWithContent.content = "export * from './home'\nexport * from './auth'" | ||
} else if (child.path === joinFromRoot('entities', 'user', 'ui', 'index.ts')) { | ||
fileWithContent.content = "export { UserCard } from './UserCard'" | ||
} else { | ||
fileWithContent.content = '' | ||
} | ||
} else if (child.type === 'folder') { | ||
addContentToFiles(child) | ||
} | ||
} | ||
} | ||
|
||
addContentToFiles(root) | ||
|
||
expect(noWildcardExports.check(root)).toEqual({ diagnostics: [] }) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
praise: kudos for using the oxc-parser :) I was hoping that we would switch to it entirely at some point and drop
precinct
because it isn't flexible enough and doesn't extend wellThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way, it supposedly provides a list of exports in a separate list that's easily accessible, can we use that? See "Returns ESM information" in https://www.npmjs.com/package/oxc-parser