-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add Claude skills for business logic separation and no-barrel-exports #6279
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: main
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,45 @@ | ||
| --- | ||
| name: no-barrel-exports | ||
| description: > | ||
| Enforce direct imports/exports instead of barrel (index.ts) files. | ||
| Use when writing new code, creating new components, adding new modules, | ||
| or refactoring imports in this repository. Prevents creation of index.ts | ||
| or index.tsx files that only re-export from other files. | ||
| --- | ||
|
|
||
| # No Barrel Exports | ||
|
|
||
| ## Rule | ||
|
|
||
| Never create `index.ts` or `index.tsx` files that re-export from other files. Import directly from the source file. | ||
|
|
||
| ```typescript | ||
| // ❌ Don't create index.ts barrel files | ||
| // components/index.ts | ||
| export { Button } from "./Button"; | ||
| export { Input } from "./Input"; | ||
|
|
||
| // ❌ Don't import from directory (relies on index.ts) | ||
| import { Button } from "./components"; | ||
| import { ProductList } from "@dashboard/products"; | ||
|
|
||
| // ✅ Import directly from the file | ||
| import { Button } from "./components/Button"; | ||
| import { ProductList } from "@dashboard/products/components/ProductList"; | ||
| ``` | ||
|
|
||
| ## When Creating New Files | ||
|
|
||
| - Name the file after its primary export: `ProductList.tsx`, `useProductForm.ts`, `productUtils.ts` | ||
| - Place the file in the appropriate feature directory | ||
| - Export named exports directly from the file — no `index.ts` wrapper | ||
|
|
||
| ## When Adding Exports to Existing Modules | ||
|
|
||
| - Add the export in the source file, not in an index file | ||
| - Update imports at call sites to point to the source file directly | ||
|
|
||
| ## Exceptions | ||
|
|
||
| - `index.tsx` files that are **route entry points** (e.g., a page component that IS the index) are fine — they contain real code, not just re-exports | ||
| - Existing `index.ts` barrel files should not be deleted as part of unrelated work — that is a separate refactoring task |
|
Member
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: in general maybe we should also add instructions about finding existing utilities? I think we have plenty of them (e.g. related to prices) - it would be great if we would re-use that instead of creating more and more code |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,127 @@ | ||||||
| --- | ||||||
| name: separate-business-logic | ||||||
| description: > | ||||||
| Extract business logic from React components into separate, testable files. | ||||||
| Use when refactoring components that have inlined data transformations, mapping, | ||||||
| filtering, sorting, formatting, validation, conditional logic, or computations | ||||||
| mixed into JSX markup. Also use when asked to improve testability of a component | ||||||
| or move logic out of a React component. Also use when asked to create new React components. | ||||||
| --- | ||||||
|
|
||||||
| # Separate Business Logic | ||||||
|
|
||||||
| ## Goal | ||||||
|
|
||||||
| Extract business logic out of React components into standalone, testable files. Components should focus on rendering and user interaction, delegating data transformations, mapping, filtering, sorting, formatting, validation, and decision-making to separate modules. | ||||||
|
|
||||||
| ## What Counts as Business Logic | ||||||
|
|
||||||
| - Data mapping and transformations (e.g., converting API responses to UI models) | ||||||
|
Member
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 add a description so that it doesn't overengineer data mapping - a simple |
||||||
| - Filtering, sorting, grouping collections | ||||||
| - Validation rules and error message resolution | ||||||
| - Conditional logic that determines what to display or which action to take | ||||||
| - Formatting (dates, currencies, percentages, labels) | ||||||
|
Member
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: we should use |
||||||
| - Computations and aggregations | ||||||
| - State machine transitions or workflow steps | ||||||
|
|
||||||
| ## Where to Put It | ||||||
|
|
||||||
| Place logic in a separate file **next to the component** that uses it: | ||||||
|
|
||||||
| ``` | ||||||
| ComponentName/ | ||||||
| ├── ComponentName.tsx # UI only | ||||||
| ├── useComponentName.ts # Hook (optional, bridges logic → component) | ||||||
| ├── componentNameUtils.ts # Pure logic functions | ||||||
| ├── componentNameUtils.test.ts # Unit tests for the logic | ||||||
| └── ComponentName.module.css | ||||||
|
Member
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. question: do we want to move ahead with writing |
||||||
| ``` | ||||||
|
|
||||||
| - **`*Utils.ts`** — set of pure functions or a cohesive class. No React imports needed. | ||||||
| - **`use*.ts`** — a custom hook, when logic needs React context, state, or effects. The hook should call into the utils rather than inlining logic. | ||||||
| - Components import from utils directly, or consume them through the hook. | ||||||
|
|
||||||
| ## Rules | ||||||
|
|
||||||
| 1. **No logic inlined in JSX/HTML markup.** Move ternaries, maps with transformations, and computed values out of the `return` block into named variables, functions, or the hook. | ||||||
|
|
||||||
| 2. **Pure functions first.** If logic doesn't need React state, context, or lifecycle — write it as a plain function in `*Utils.ts`. These are the easiest to test. | ||||||
|
|
||||||
| 3. **Hooks wrap context-dependent logic.** When logic requires `useContext`, `useState`, or other hooks, create a `use*.ts` hook. Keep the hook thin — delegate heavy work to util functions and pass results back. | ||||||
|
|
||||||
| 4. **Pattern: logic → hook → component.** | ||||||
|
|
||||||
| ``` | ||||||
| // utils: pure logic | ||||||
| export const calculateTotal = (items: Item[]): number => { ... } | ||||||
|
|
||||||
| // hook: connects React world to logic | ||||||
| export const useCart = () => { | ||||||
| const items = useContext(CartContext); | ||||||
| const total = useMemo(() => calculateTotal(items), [items]); | ||||||
| return { items, total }; | ||||||
| }; | ||||||
|
|
||||||
| // component: renders | ||||||
| export const Cart = () => { | ||||||
| const { items, total } = useCart(); | ||||||
| return <Box>...</Box>; | ||||||
| }; | ||||||
| ``` | ||||||
|
|
||||||
| 5. **Test the logic, not the wiring.** Unit test `*Utils.ts` functions thoroughly — edge cases, empty inputs, error states. Hook tests and component tests can be lighter since they just verify integration. | ||||||
|
|
||||||
| 6. **Name functions by intent.** Use descriptive names: `getActiveFilters`, `formatPriceRange`, `resolveStatusBadgeColor` — not `processData` or `transform`. | ||||||
|
|
||||||
| ## Testing Expectations | ||||||
|
|
||||||
| - Every `*Utils.ts` file must have a corresponding `*.test.ts` file. | ||||||
|
Member
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 add explicitly hooks
Suggested change
|
||||||
| - Tests should use `// Arrange // Act // Assert` comments. | ||||||
| - Declare fixture types explicitly: `const fixture: FixtureType = { ... }`. | ||||||
| - Cover: happy path, empty/null inputs, boundary values, and error cases. | ||||||
|
|
||||||
| ```typescript | ||||||
| // calculateTotal.test.ts | ||||||
| import { calculateTotal } from "./cartUtils"; | ||||||
| import { Item } from "./types"; | ||||||
|
|
||||||
| describe("calculateTotal", () => { | ||||||
| it("sums item prices multiplied by quantity", () => { | ||||||
| // Arrange | ||||||
| const items: Item[] = [ | ||||||
| { price: 10, quantity: 2 }, | ||||||
| { price: 5, quantity: 3 }, | ||||||
| ]; | ||||||
|
|
||||||
| // Act | ||||||
| const result = calculateTotal(items); | ||||||
|
|
||||||
| // Assert | ||||||
| expect(result).toBe(35); | ||||||
| }); | ||||||
|
|
||||||
| it("returns 0 for empty list", () => { | ||||||
| // Arrange | ||||||
| const items: Item[] = []; | ||||||
|
|
||||||
| // Act | ||||||
| const result = calculateTotal(items); | ||||||
|
|
||||||
| // Assert | ||||||
| expect(result).toBe(0); | ||||||
| }); | ||||||
| }); | ||||||
| ``` | ||||||
|
Comment on lines
+83
to
+114
Member
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: maybe we should separate examples into a separate file in a skill directory? This is a good way to save context for specific use cases, e.g. writing test |
||||||
|
|
||||||
| ## Applying This Skill | ||||||
|
|
||||||
| When asked to separate business logic: | ||||||
|
|
||||||
| 1. Read the target component(s) to identify inlined logic. | ||||||
| 2. Catalog each piece of logic (transformations, conditionals, formatting, etc.). | ||||||
| 3. Create `*Utils.ts` with extracted pure functions. | ||||||
| 4. If React context/state is needed, create or update a `use*.ts` hook that uses the utils. | ||||||
| 5. Update the component to import from utils / hook — remove inlined logic. | ||||||
| 6. Write unit tests for every extracted function. | ||||||
| 7. Run `pnpm run test:quiet <test_file>` to verify. | ||||||
| 8. Run `pnpm run lint` and `pnpm run check-types` to ensure nothing broke. | ||||||
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.
suggestion: for enforcement SKILLS are brittle, claude loads them dynamically - it might load or not and it can just straight up ignore it 😄. It's better to use them to perform certain tasks in a way we want (e.g. separate business logic)
Maybe we could rewrite this to a hook instead? This way we can check if Claude edits / creates
index.ts/index.tsxand return an error there to force it to not create such files programaticallyExample:
saleor-dashboard/.claude/hooks/check-as-any-pre.py
Line 4 in 57cef53