-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat(skill): make react-clean-architecture-add-component self-contained with local references #1910
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
Open
tmt823
wants to merge
10
commits into
github:staged
Choose a base branch
from
tmt823:feat/react-clean-architecture-add-component-skill
base: staged
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+270
−0
Open
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
5433650
feat(skill): make react-clean-architecture-add-component self-contain…
tmt823 9b9408c
docs(skill): clarify SCSS rules for z-index tokens and Mantine comple…
tmt823 3ef1eb9
docs(skill): remove ambiguous "when needed" wording for presentation …
tmt823 2243c97
docs(skill): generalize build/lint validation wording
tmt823 464485f
refactor(skill): generalize component scaffolding guidance beyond Man…
tmt823 4717bd0
fix: component-architecture.md
tmt823 1f072db
feat(skill): rename react component skill and clarify container/prese…
tmt823 4f454b5
fix(skill): replace vscode_askQuestions with ask_user to remove VS Co…
tmt823 bae1b9d
docs(skill): add TypeScript code examples to typescript-and-scss-rule…
tmt823 4feeb93
fix: README
tmt823 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| --- | ||
| name: react-container-presentation-component | ||
| description: "Create a React component using the Container/Presentation pattern in src/components by asking for the component name and type (ui or features), then scaffold files that follow this repository's TypeScript, Storybook, and SCSS conventions. Use when the user explicitly asks for a Container/Presentation-based component or runs /react-container-presentation-component." | ||
| argument-hint: "componentName type(ui|features)" | ||
| user-invocable: true | ||
| --- | ||
|
|
||
| # Container/Presentation Component | ||
|
|
||
| Use this skill to create a React component under `src/components` that follows the Container/Presentation pattern. | ||
|
|
||
| Refer to this skill's bundled references for detailed rules. | ||
|
|
||
| - `references/component-architecture.md` | ||
| - `references/typescript-and-scss-rules.md` | ||
|
|
||
| If the `/react-container-presentation-component` input is incomplete, ask questions first before creating files. | ||
|
|
||
| ## When To Use | ||
|
|
||
| - When the user runs `/react-container-presentation-component` | ||
| - When the user explicitly asks for a React component that follows the Container/Presentation pattern | ||
| - When the user wants help deciding or implementing `ui` vs `features` classification within the Container/Presentation pattern | ||
|
|
||
| ## Required Questions | ||
|
|
||
| If any of the following information is missing, ask the user using `ask_user`. | ||
|
|
||
| 1. Component name | ||
| 2. Type (`ui` or `features`) | ||
| 3. Whether to replace existing components (only when creating `ui`) | ||
|
|
||
| Question requirements: | ||
|
|
||
| - Provide the type as options (`ui`, `features`) | ||
| - Require the component name to be in PascalCase | ||
| - For `ui`, ask whether direct usage of Mantine or other UI libraries in existing `features` should be replaced with the new component | ||
|
|
||
| ## Procedure | ||
|
|
||
| 1. Check existing components | ||
|
|
||
| - Check whether `src/components/ui/<ComponentName>` or `src/components/features/<ComponentName>` already exists. | ||
| - If it exists, do not overwrite; confirm the preferred approach with the user. | ||
|
|
||
| 2. Decide target directory | ||
|
|
||
| - `ui`: `src/components/ui/<ComponentName>` | ||
| - `features`: `src/components/features/<ComponentName>` | ||
|
|
||
| 3. Re-check classification (only when `ui` is specified) | ||
|
|
||
| - Even when `ui` is specified, before creating files, review `Reclassification Rule` in `references/component-architecture.md`. | ||
| - If the implementation includes state management, side effects, async processing, context/store updates, or business logic, treat it as `features`. | ||
| - If the result is closer to `features`, do not proceed as `ui`; use `ask_user` and confirm one of the following before continuing. | ||
| - `Create as features` | ||
| - `Keep ui and move state/logic to parent or features` | ||
|
|
||
| 4. Create required files | ||
|
|
||
| - `ui`: `index.tsx`, `index.module.scss`, `index.stories.tsx` | ||
| - `features`: `index.tsx`, `use<ComponentName>.tsx`, `presentation.tsx`, `types.ts`, `presentation.module.scss`, `presentation.stories.tsx` | ||
|
|
||
| 5. Replace existing usages (only when creating `ui`) | ||
|
|
||
| - Only when the user approves, replace equivalent direct implementations using Mantine or other UI libraries in existing `features` with the new `ui` component. | ||
|
|
||
| 6. Validate | ||
|
|
||
| - Run build and lint commands, and ensure both pass; if issues are introduced by newly added or updated files, fix them. | ||
| - Follow `Storybook Minimum` in `references/component-architecture.md` for story state decisions. | ||
| - Ask the user via `ask_user` whether to run a Storybook check (for example: "Run" / "Skip for now"). | ||
| - Run `npm run storybook` only if the user selects "Run". | ||
| - If the user selects "Skip for now", explicitly mention in the final report that Storybook execution was skipped. | ||
|
|
||
| ## Output Contract | ||
|
|
||
| - Report the list of files created. | ||
| - If replacements were performed, report the list of changed files and replacement details. | ||
| - Provide one usage example of the created component. | ||
| - Report whether Storybook verification was executed (run/skip), and if run, include the command used. | ||
| - Explain why the component was classified as `ui` or `features`. | ||
| - Summarize where state, side effects, and rendering responsibilities were placed. | ||
| - Confirm whether there were any dependency direction violations. | ||
| - Clearly state any unresolved items. | ||
130 changes: 130 additions & 0 deletions
130
skills/react-container-presentation-component/references/component-architecture.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,130 @@ | ||
| # Component Architecture Reference | ||
|
|
||
| This reference defines classification, file layout, and dependency direction in `src/components`. | ||
|
|
||
| ## Design Intent and Principles | ||
|
|
||
| The goal of this skill is not only to add React components, but to apply the Container/Presentation pattern consistently with clear separation of responsibilities and dependency direction. | ||
|
|
||
| This reference does not define full application-wide architecture. It focuses on design quality at the component boundary. | ||
|
|
||
| - Separate rendering responsibilities from logic responsibilities. | ||
| - Do not place state management, side effects, or business decisions in the presentation layer. | ||
| - Avoid mixing responsibilities across boundaries and keep dependency direction explicit. | ||
|
|
||
| ## Classification | ||
|
|
||
| - Place all components under `src/components`. | ||
| - Use only two categories: | ||
| - `ui`: render-only, stateless components. | ||
| - `features`: components that include logic. | ||
|
|
||
| ## Reclassification Rule | ||
|
|
||
| If the user requests `ui` but the implementation contains any of the following, treat it as `features` and ask for confirmation before creating files: | ||
|
|
||
| - `useState`, `useReducer`, or `useEffect`. | ||
| - Async behavior (API calls, timers, subscriptions). | ||
| - Reading from or writing to context/store. | ||
| - Business/data transformation logic. | ||
|
|
||
| Ask using these options: | ||
|
|
||
| - `Create as features` | ||
| - `Keep ui and move logic/state to parent or features` | ||
|
|
||
| ## Layer Responsibilities | ||
|
|
||
| This skill defines layers in two stages. | ||
|
|
||
| ### 1. Component Type Layer | ||
|
|
||
| | Type | Responsibility | | ||
| | ---------- | ------------------------------------------------------------------------------------------------------ | | ||
| | `ui` | Reusable render-only component. Must not include business logic, side effects, or state management. | | ||
| | `features` | Use-case-oriented component. Handles state transitions, event interpretation, and async orchestration. | | ||
|
|
||
| ### 2. Internal Layer in `features` | ||
|
|
||
| | Layer | Responsibility | Primary files | | ||
| | -------------- | ----------------------------------------------------------------------------------- | -------------------------------------------------------------------------- | | ||
| | `container` | Handles state management, side effects, event handling, and data fetching. | `index.tsx`, `use<ComponentName>.tsx`, `types.ts` | | ||
| | `presentation` | Receives props and renders UI only. Must not perform external I/O or state updates. | `presentation.tsx`, `presentation.module.scss`, `presentation.stories.tsx` | | ||
|
|
||
| Notes: | ||
|
|
||
| - `ui` is composed of presentation only. | ||
| - `features` must separate container and presentation. | ||
|
|
||
| ## Implementation Rules | ||
|
|
||
| ### ui | ||
|
|
||
| - Keep components stateless. | ||
| - Accept data and callbacks via props. | ||
| - Do not add side effects or data fetching. | ||
| - Prefer primitives from Mantine or other UI libraries first; use custom JSX/SCSS only when needed. | ||
|
|
||
| ### features | ||
|
|
||
| - Use the Container/Presentation pattern. | ||
| - Keep logic in `use<ComponentName>.tsx`. | ||
| - Follow `Container/Presentation Separation Rules (Anti-patterns and Decision Examples)` below for detailed responsibility boundaries and anti-patterns. | ||
|
|
||
| ### Container/Presentation Separation Rules (Anti-patterns and Decision Examples) | ||
|
|
||
| Principles: | ||
|
|
||
| - Container is responsible for state management, side effects, event interpretation, and async processing. | ||
| - Presentation is responsible only for rendering from received props. | ||
| - Keep business decisions and data transformation in container-side code, not in presentation. | ||
|
|
||
| Placement rules: | ||
|
|
||
| - Place in container: `useState` / `useReducer` / `useEffect`, API calls, context/store read-write, business rule application. | ||
| - Place in presentation: JSX rendering and display-only branching (for example: empty, loading, error views). | ||
| - Use `types.ts` to define I/O contracts between container and presentation. | ||
|
|
||
| Anti-patterns: | ||
|
|
||
| - Calling APIs or mutations from presentation. | ||
| - Updating context/store directly from presentation. | ||
| - Implementing business decisions (authorization checks, state transition decisions, data shaping) in presentation. | ||
| - Splitting files formally while keeping practical logic in presentation. | ||
|
|
||
| Good / Bad examples: | ||
|
|
||
| - Bad: `presentation.tsx` fetches data and manages loading state directly. | ||
| - Good: `use<ComponentName>.tsx` manages data fetching and state, and `presentation.tsx` renders only from props such as `isLoading`, `items`, and `onAction`. | ||
|
|
||
| ## Dependency Direction | ||
|
|
||
| - `features` -> `ui`: allowed. | ||
| - `ui` -> `features`: forbidden. | ||
|
|
||
| ## File Structure | ||
|
|
||
| ### ui | ||
|
|
||
| - `index.tsx` | ||
| - `presentation.tsx` | ||
| - `presentation.stories.tsx` | ||
| - `presentation.module.scss` | ||
|
|
||
| ### features | ||
|
|
||
| - `index.tsx` | ||
| - `use<ComponentName>.tsx` | ||
| - `presentation.tsx` | ||
| - `types.ts` | ||
| - `presentation.stories.tsx` | ||
| - `presentation.module.scss` | ||
|
|
||
| ## Storybook Minimum | ||
|
|
||
| - Always create `Default`. | ||
| - Add state-specific stories only when distinct states exist. | ||
| - Prefer story sets based on behavior: | ||
| - Interactive controls: `Hover`. | ||
| - Input-like: `Focus`, `Error`, `Disabled`. | ||
| - Layout/open-close: `Open`, `Closed`, `Empty`. |
54 changes: 54 additions & 0 deletions
54
.../react-container-presentation-component/references/typescript-and-scss-rules.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| # TypeScript and SCSS Rules Reference | ||
|
|
||
| ## TypeScript Rules | ||
|
|
||
| - Do not use `any`. | ||
|
|
||
| ```ts | ||
| // Bad | ||
| const handler = (e: any) => {}; | ||
|
|
||
| // Good | ||
| const handler = (e: React.ChangeEvent<HTMLInputElement>) => {}; | ||
| ``` | ||
|
|
||
| - Use `type` for props instead of `interface`. | ||
|
|
||
| ```ts | ||
| // Bad | ||
| interface ButtonProps { | ||
| label: string; | ||
| } | ||
|
|
||
| // Good | ||
| type ButtonProps = { label: string }; | ||
| ``` | ||
|
|
||
| - Explicitly annotate function return types. | ||
|
|
||
| ```ts | ||
| // Bad | ||
| const getLabel = () => "hello"; | ||
|
|
||
| // Good | ||
| const getLabel = (): string => "hello"; | ||
| ``` | ||
|
|
||
| ## SCSS Rules | ||
|
|
||
| ### Tokens | ||
|
|
||
| - Use color variables from `src/styles/theme.scss`. | ||
| - Use animation variables from `src/styles/animation.scss`. | ||
| - Define z-index tokens in `src/styles/z-index.scss` and consume those tokens in component styles. | ||
| - In component SCSS, do not hardcode z-index values (for example, avoid `z-index: 10;` and use a z-index token instead). | ||
|
|
||
| ### Style Constraints | ||
|
|
||
| - Prefer Mantine or other UI libraries first; use SCSS only when complementing library styles is necessary. | ||
| - Do not use negative margins. | ||
| - Prefer unitless `line-height`. | ||
| - Prefer `letter-spacing` in `em`. | ||
| - When margin is needed, only `margin-top` and `margin-left` are allowed. | ||
| - Do not set `margin` or `position` on root elements. | ||
| - The numeric values inside `src/styles/z-index.scss` must follow a 50-step scale (100, 150, ...). |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.