-
Notifications
You must be signed in to change notification settings - Fork 5
Refactoring #91
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
Refactoring #91
Conversation
…...CalledExactlyOnceWith`
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@barso/forms
@barso/helpers
@barso/hooks
@barso/infra
@barso/ui
commit: |
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.
Pull request overview
This PR refactors multiple files across the codebase to improve code quality and maintainability. The changes include updating test assertions to use more concise Vitest matchers, converting mock functions to proper constructor functions, simplifying React components by removing unnecessary state management, and replacing mutations with immutable patterns.
Key changes:
- Converted arrow function mocks to constructor functions for proper class instantiation behavior
- Updated test assertions to use
toHaveBeenCalledOnce()andtoHaveBeenCalledExactlyOnceWith()for improved readability - Refactored
useTreeCollapsehook fromuseStatetouseReducerfor better state management - Simplified
MarkdownViewerby removing unnecessary state management - Replaced object mutation with immutable spread operator in
SubmittedFields - Added
adoptedStyleSheetspolyfill for jsdom compatibility
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/setup.js | Added adoptedStyleSheets polyfill for jsdom environment to fix compatibility issue |
| packages/ui/src/_document.test.js | Converted ServerStyleSheet mock from arrow function to constructor function for proper instantiation |
| packages/ui/src/Notifications/NotificationMenu.test.jsx | Updated test assertion to use toHaveBeenCalledOnce() |
| packages/ui/src/Notifications/NotificationList.test.jsx | Updated test assertions to use toHaveBeenCalledOnce() |
| packages/ui/src/Markdown/Markdown.jsx | Removed unnecessary state management in MarkdownViewer, replaced Box with div, removed unused imports |
| packages/ui/src/GoToTopButton/GoToTopButton.test.jsx | Updated mock to constructor function and test assertions to use toHaveBeenCalledExactlyOnceWith() |
| packages/ui/src/FormField/FormField.test.js | Updated test assertion to use toHaveBeenCalledOnce() |
| packages/ui/src/FormField/FormField.jsx | Moved ref assignment after spread operator to prevent accidental override |
| packages/infra/src/logger/logger.test.js | Converted Axiom mock from arrow function to constructor function and hoisted flush mock |
| packages/infra/src/logger/axiom-transport.test.js | Converted Axiom mocks to constructor functions and updated test assertions |
| packages/hooks/src/useTreeCollapse/useTreeCollapse.js | Refactored from useState/useCallback to useReducer for cleaner state management |
| packages/helpers/src/dom.test.js | Updated test assertion to use toHaveBeenCalledOnce() |
| examples/form/components/SubmittedFields.jsx | Replaced delete mutation with immutable spread operator pattern |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.