chore: bugbot #3206
chore: bugbot #3206
Conversation
✅ No New Circular DependenciesNo new circular dependencies detected. Current count: 0 |
📦 Alpha Package Version PublishedUse Use |
🔍 Visual review for your branch is published 🔍Here are the links to: |
Coverage Report for packages/react
File CoverageNo changed files found. |
There was a problem hiding this comment.
Pull request overview
This PR introduces automated code review guidelines for the React package and enhances the pre-commit workflow to enforce formatting and linting standards.
- Adds comprehensive code review guidelines in
BUGBOT.mdcovering component structure, TypeScript standards, testing, styling, and Storybook requirements - Updates package.json scripts to accept optional file path arguments, enabling more flexible linting and formatting workflows
- Configures lefthook to automatically format and lint staged React files on commit with path rewriting
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 12 comments.
| File | Description |
|---|---|
| packages/react/BUGBOT.md | Adds detailed code review guidelines for React components, TypeScript, testing, styling, accessibility, and Storybook |
| packages/react/package.json | Modifies lint and format scripts to accept optional file arguments using shell parameter expansion |
| lefthook.yml | Updates format-react hook and adds lint-react hook with path transformation to strip package prefix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "lint": "sh -c 'oxlint \"${@:-src/**/*}\" --max-warnings 0' --", | ||
| "lint:fix": "sh -c 'oxlint \"${@:-src/**/*}\" --fix --max-warnings 0' --", |
There was a problem hiding this comment.
The shell parameter expansion ${@:-src/**/*} may not work correctly in all POSIX-compliant shells when invoked with sh -c. While this syntax works in bash, it's not standard POSIX. Consider using a more portable approach like: if [ $# -eq 0 ]; then set -- src/**/*; fi; oxlint "$@" --max-warnings 0 or explicitly use bash -c instead of sh -c.
| "format": "sh -c 'oxfmt \"${@:-src/}\"' --", | ||
| "format:check": "sh -c 'oxfmt --check \"${@:-src/}\"' --", |
There was a problem hiding this comment.
The shell parameter expansion ${@:-src/} may not work correctly in all POSIX-compliant shells when invoked with sh -c. While this syntax works in bash, it's not standard POSIX. Consider using a more portable approach like: if [ $# -eq 0 ]; then set -- src/; fi; oxfmt "$@" or explicitly use bash -c instead of sh -c.
| ## Storybook | ||
| - Ensure the components have at leat one story file |
There was a problem hiding this comment.
Typo: "at leat" should be "at least"
| - Ensure the components have at leat one story file | |
| - Ensure the components have at least one story file |
There was a problem hiding this comment.
Nice suggestion by Copilot. Please add it
| - Ensure the components have at leat one story file | ||
| - Ensure the story docs covers all the properties | ||
| - For properties with with limited and well know options use the const array for the select values from the component. Avoid repeating the values in the story |
There was a problem hiding this comment.
Typo: "with with" should be "with"
| - For properties with with limited and well know options use the const array for the select values from the component. Avoid repeating the values in the story | |
| - For properties with limited and well know options use the const array for the select values from the component. Avoid repeating the values in the story |
There was a problem hiding this comment.
Nice suggestion by Copilot. Please add it
| ### `src/ui/` | ||
| Ensure those components are no reexported |
There was a problem hiding this comment.
Grammatical error: "no reexported" should be "not re-exported"
| Ensure those components are no reexported | |
| Ensure those components are not re-exported |
There was a problem hiding this comment.
Nice suggestion by Copilot. Please add it
| - Ensure the components have at leat one story file | ||
| - Ensure the story docs covers all the properties | ||
| - For properties with with limited and well know options use the const array for the select values from the component. Avoid repeating the values in the story |
There was a problem hiding this comment.
Typo: "know" should be "known"
| - For properties with with limited and well know options use the const array for the select values from the component. Avoid repeating the values in the story | |
| - For properties with with limited and well known options use the const array for the select values from the component. Avoid repeating the values in the story |
There was a problem hiding this comment.
Nice suggestion by Copilot. Please add it
| } | ||
| ``` | ||
| - Ensure there is a story called Snapshot which includes `parameters: withSnapshot({}),` to ensure the component has visual regresions tests in chromatic |
There was a problem hiding this comment.
Typo: "regresions" should be "regressions"
| - Ensure there is a story called Snapshot which includes `parameters: withSnapshot({}),` to ensure the component has visual regresions tests in chromatic | |
| - Ensure there is a story called Snapshot which includes `parameters: withSnapshot({}),` to ensure the component has visual regression tests in chromatic |
There was a problem hiding this comment.
Nice suggestion by Copilot. Please add it
| - `hooks/` if the component has hook files (`useXXXX.ts`) should be in this folder | ||
| - `components/` if the component have internal subcomponents should be in this folder | ||
|
|
||
| Ensure elements `componentes/`, `internal-types` and `internal` files are not exported |
There was a problem hiding this comment.
Grammatical error: "Ensure elements componentes/, internal-types and internal files are not exported" is unclear. Consider revising to: "Ensure elements in components/, internal-types.ts, and internal files are not exported"
| Ensure elements `componentes/`, `internal-types` and `internal` files are not exported | |
| Ensure elements in `components/`, `internal-types.ts`, and `internal` files are not exported |
There was a problem hiding this comment.
Nice suggestion by Copilot. Please add it
|
|
||
| ## Component Structure | ||
|
|
||
| - ban new components in `experimental` folder, they should be in `components` folder and use the experimentalComponent function from `@/lib/experimental.ts` |
There was a problem hiding this comment.
The directive "ban new components in experimental folder" is unclear. Consider rephrasing to: "New components should not be added to the experimental folder; instead, add them to the components folder and use the experimentalComponent function from @/lib/experimental.ts"
| - ban new components in `experimental` folder, they should be in `components` folder and use the experimentalComponent function from `@/lib/experimental.ts` | |
| - New components should not be added to the `experimental` folder; instead, add them to the `components` folder and use the experimentalComponent function from `@/lib/experimental.ts` |
There was a problem hiding this comment.
Nice suggestion by Copilot. Please add it
developerdanx
left a comment
developerdanx
left a comment
There was a problem hiding this comment.
Thank you for bringing new improvements. Copilot left some nice comments and suggestions.
|
|
||
| ## Component Structure | ||
|
|
||
| - ban new components in `experimental` folder, they should be in `components` folder and use the experimentalComponent function from `@/lib/experimental.ts` |
There was a problem hiding this comment.
Nice suggestion by Copilot. Please add it
| - `hooks/` if the component has hook files (`useXXXX.ts`) should be in this folder | ||
| - `components/` if the component have internal subcomponents should be in this folder | ||
|
|
||
| Ensure elements `componentes/`, `internal-types` and `internal` files are not exported |
There was a problem hiding this comment.
Nice suggestion by Copilot. Please add it
| ### `src/ui/` | ||
| Ensure those components are no reexported |
There was a problem hiding this comment.
Nice suggestion by Copilot. Please add it
| ## Storybook | ||
| - Ensure the components have at leat one story file |
There was a problem hiding this comment.
Nice suggestion by Copilot. Please add it
| - Ensure the components have at leat one story file | ||
| - Ensure the story docs covers all the properties | ||
| - For properties with with limited and well know options use the const array for the select values from the component. Avoid repeating the values in the story |
There was a problem hiding this comment.
Nice suggestion by Copilot. Please add it
| - Ensure the components have at leat one story file | ||
| - Ensure the story docs covers all the properties | ||
| - For properties with with limited and well know options use the const array for the select values from the component. Avoid repeating the values in the story |
There was a problem hiding this comment.
Nice suggestion by Copilot. Please add it
| } | ||
| ``` | ||
| - Ensure there is a story called Snapshot which includes `parameters: withSnapshot({}),` to ensure the component has visual regresions tests in chromatic |
There was a problem hiding this comment.
Nice suggestion by Copilot. Please add it
Co-authored-by: Copilot <[email protected]>
Description
Screenshots (if applicable)
[Link to Figma Design](Figma URL here)
Implementation details
Note
Introduces automated review guidance and improves developer workflow for the React package.
packages/react/BUGBOT.mdwith comprehensive code review rules BugBot will enforcelefthook.ymlpre-commit for React:format-reactnow calls packageformaton staged paths (repo-relative → package-relative) and addslint-reactto runlint:fixon staged filespackages/react/package.jsonscripts:lint/lint:fixandformat/format:checknow accept optional path args (defaulting tosrc/**/*orsrc/), replacing hardcoded targetsWritten by Cursor Bugbot for commit 133b11b. This will update automatically on new commits. Configure here.