Refactoring #7
Refactoring #7
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the codebase with comprehensive improvements including code style standardization, test coverage expansion, documentation enhancements, and infrastructure updates.
Changes:
- Migrated build tooling from Vite to tsup for library bundling with dual ESM/CJS output
- Added comprehensive unit and E2E test suites with Vitest and Playwright
- Standardized code formatting (Prettier) and linting (ESLint) across all files
- Enhanced public API with improved type exports and documentation
Reviewed changes
Copilot reviewed 87 out of 98 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Expanded test configuration with jsdom environment, coverage thresholds, and test directory structure |
| vite.config.ts | Quote style standardization (double → single) |
| tsup.config.ts | Added file extension control for ESM/CJS outputs |
| tests/unit/**/*.test.tsx | New comprehensive test suites for hooks, components, and utilities |
| tests/e2e/**/*.spec.ts | New E2E tests for examples application |
| tests/setup.ts | Test environment setup with jest-dom matchers |
| src/hooks/* | New authentication and accessibility hooks with TypeScript types |
| src/components/* | Code formatting improvements and documentation additions |
| src/index.ts | Expanded public API exports with comprehensive type definitions |
| package.json | Updated dependencies, scripts, and build configuration |
| examples/* | New examples application demonstrating library usage |
| eslint.config.mjs | New ESLint flat config with TypeScript and React rules |
| playwright.config.ts | New Playwright configuration for E2E testing |
| .prettierrc | New Prettier configuration for consistent formatting |
| .husky/* | Updated Git hooks for pre-commit and pre-push validation |
| .github/workflows/* | New CI/CD workflows for testing and releases |
| docs/* | Enhanced documentation for architecture and release processes |
| README.md | Expanded usage examples and API documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -250,7 +242,7 @@ export default function ControlledZodDynamicForm({ | |||
| //Show erros validation | |||
There was a problem hiding this comment.
Corrected spelling of 'erros' to 'errors' in comment.
| //Show erros validation | |
| // Show errors validation |
| @@ -1,19 +1,25 @@ | |||
| import React from 'react'; | |||
| import { Link } from 'react-router'; | |||
| import { Link } from 'react-router-dom'; | |||
There was a problem hiding this comment.
Import changed from 'react-router' to 'react-router-dom', but package.json shows both as peer dependencies. Consider documenting which package consumers should install, or standardize on one router package across the codebase.
| import { Link } from 'react-router-dom'; | |
| import { Link } from 'react-router'; |
| useT: () => (key: string, vars?: any) => (typeof key === 'string' ? key : 't'), | ||
| })); | ||
|
|
||
| vi.mock('react-router', () => ({ |
There was a problem hiding this comment.
Test mocks 'react-router' but the component imports from 'react-router-dom'. This mock will not affect the component under test. Update the mock path to 'react-router-dom' to match the import.
| vi.mock('react-router', () => ({ | |
| vi.mock('react-router-dom', () => ({ |
| { key: 'name', label: 'Name', type: 'text' }, | ||
| { key: 'email', label: 'Email', type: 'email' }, |
There was a problem hiding this comment.
Field configuration uses 'key' property but FieldConfigDynamicForm type expects 'name' property. According to the type definition, fields should use 'name' not 'key'.
| { key: 'name', label: 'Name', type: 'text' }, | |
| { key: 'email', label: 'Email', type: 'email' }, | |
| { name: 'name', label: 'Name', type: 'text' }, | |
| { name: 'email', label: 'Email', type: 'email' }, |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 85 out of 96 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, [container, selector, initialIndex]); | ||
| } | ||
|
|
||
| export default undefined; |
There was a problem hiding this comment.
Exporting undefined as default export serves no purpose and can be confusing. Either remove the default export entirely or export the hook function as default.
| export default undefined; | |
| export default useKeyboardNavigation; |
|
|
||
| export default undefined; |
There was a problem hiding this comment.
Exporting undefined as default export serves no purpose and can be confusing. Either remove the default export entirely or export the named functions as a single default object.
| export default undefined; |
1 participant
Summary
Why
Checklist
npm run lintpassesnpm run typecheckpassesnpm testpassesnpm run buildpassesnpx changeset) if this affects consumersNotes