Skip to content

chore(husky): remove deprecated bootstrap lines; style: run prettier …#5

Closed
a-elkhiraooui-ciscode wants to merge 5 commits intomasterfrom
refactoring
Closed

chore(husky): remove deprecated bootstrap lines; style: run prettier …#5
a-elkhiraooui-ciscode wants to merge 5 commits intomasterfrom
refactoring

Conversation

@a-elkhiraooui-ciscode
Copy link
Contributor

…write

Summary

  • What does this PR change?

Why

  • Why is this change needed?

Checklist

  • Added/updated tests (if behavior changed)
  • npm run lint passes
  • npm run typecheck passes
  • npm test passes
  • npm run build passes
  • Added a changeset (npx changeset) if this affects consumers

Notes

  • Anything reviewers should pay attention to?

@a-elkhiraooui-ciscode a-elkhiraooui-ciscode requested review from a team and Copilot February 5, 2026 12:13
Copy link

Copilot AI left a 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 modernizes the repository's build tooling, testing infrastructure, and code quality setup. It migrates from Vite's library mode to tsup for bundling, introduces comprehensive unit and E2E testing with Vitest and Playwright, and applies consistent code formatting with Prettier.

Changes:

  • Replaced Vite library build with tsup, producing dual ESM/CJS outputs
  • Added comprehensive test coverage using Vitest (unit) and Playwright (E2E)
  • Standardized code formatting across the codebase using Prettier

Reviewed changes

Copilot reviewed 85 out of 96 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
vitest.config.ts Configured Vitest with jsdom environment, coverage thresholds, and test directories
tsup.config.ts Added tsup configuration for dual-format builds with proper file extensions
package.json Updated build scripts, added test/lint/format commands, and reorganized dependencies
src/index.ts Expanded public API exports to include all components, hooks, and types
src/components/Form/ZodDynamicForm.tsx Fixed typo 'textt' → 'text' and applied formatting
src/hooks/*.ts Added new authentication and accessibility hooks with proper TypeScript types
tests/unit/**/*.test.tsx Added comprehensive unit tests for components and hooks
tests/e2e/**/*.spec.ts Added Playwright E2E tests for examples app
.husky/pre-commit Updated to run format, lint, typecheck, and test
examples/ Added examples app with vite config for component demonstration

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

const [success, setSuccess] = useState<boolean>(false);

function update<K extends keyof PasswordResetInput>(key: K, value: PasswordResetInput[K]) {
setValues((v: PasswordResetInput) => ({ ...v, [key]: value }));
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type annotation on the updater function parameter v: PasswordResetInput is redundant since TypeScript infers it from the state type. Remove it for consistency with the .ts version in the same directory.

Suggested change
setValues((v: PasswordResetInput) => ({ ...v, [key]: value }));
setValues((v) => ({ ...v, [key]: value }));

Copilot uses AI. Check for mistakes.
const [result, setResult] = useState<LoginResult<TUser> | null>(null);

function update<K extends keyof LoginCredentials>(key: K, value: LoginCredentials[K]) {
setValues((v: LoginCredentials) => ({ ...v, [key]: value }));
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type annotation on the updater function parameter v: LoginCredentials is redundant since TypeScript infers it from the state type. Remove it for consistency with the .ts version in the same directory.

Suggested change
setValues((v: LoginCredentials) => ({ ...v, [key]: value }));
setValues((v) => ({ ...v, [key]: value }));

Copilot uses AI. Check for mistakes.
@@ -1,19 +1,25 @@
import React from 'react';
import { Link } from 'react-router';
import { Link } from 'react-router-dom';
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import changed from 'react-router' to 'react-router-dom', but the test file src/components/Breadcrumbs/Breadcrumb.test.tsx still mocks 'react-router' instead of 'react-router-dom'. This inconsistency should be resolved.

Copilot uses AI. Check for mistakes.
});
const fields: FieldConfigDynamicForm[] = [
{ key: 'name', label: 'Name', type: 'text' },
{ key: 'email', label: 'Email', type: 'email' },
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Field type 'email' is not a valid FieldType according to the model definition in src/models/FieldConfigDynamicForm.ts. The valid types are: 'text', 'number', 'textarea', 'select', 'checkbox', 'multiSelect', 'custom'. Use 'text' instead of 'email'.

Suggested change
{ key: 'email', label: 'Email', type: 'email' },
{ key: 'email', label: 'Email', type: 'text' },

Copilot uses AI. Check for mistakes.
}, [container, selector, initialIndex]);
}

export default undefined;
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exporting undefined as the default export serves no purpose. Remove this line or export the function as default if it's intended to be the primary export.

Suggested change
export default undefined;
export default useKeyboardNavigation;

Copilot uses AI. Check for mistakes.
Comment on lines +66 to +67

export default undefined;
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exporting undefined as the default export serves no purpose. Remove this line.

Suggested change
export default undefined;

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings February 5, 2026 12:28
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 86 out of 97 changed files in this pull request and generated 5 comments.


💡 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
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of 'erros' to 'errors' in comment.

Suggested change
//Show erros validation
// Show errors validation

Copilot uses AI. Check for mistakes.
@@ -1,19 +1,25 @@
import React from 'react';
import { Link } from 'react-router';
import { Link } from 'react-router-dom';
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import changed from 'react-router' to 'react-router-dom', which is correct for client-side routing. However, verify that 'react-router-dom' is listed in peerDependencies (already confirmed in package.json line 47), and ensure all other files using 'react-router' imports are also updated consistently.

Copilot uses AI. Check for mistakes.
Comment on lines +66 to +67

export default undefined;
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exporting undefined as default is unusual and may confuse consumers. Since the hooks are already exported as named exports, consider removing the default export entirely.

Suggested change
export default undefined;

Copilot uses AI. Check for mistakes.
npm run format:write
npm run lint
npm run typecheck
npm test
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running all these commands sequentially in pre-commit can be slow and frustrate developers. Consider using a tool like lint-staged to only check staged files, or move some checks (like full test suite) to pre-push or CI.

Suggested change
npm test

Copilot uses AI. Check for mistakes.
{columns.map(col => (
<th key={String(col.key)} className="px-4 py-3">{col.title}</th>
{columns.map((col, i) => (
<th key={i} className="px-4 py-3">
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using array index as key is not recommended when the column order might change. Since columns have a key property, consider using String(col.key) or a stable identifier as the key instead of the index.

Suggested change
<th key={i} className="px-4 py-3">
<th key={String(col.key)} className="px-4 py-3">

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@Zaiidmo Zaiidmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be appreciated if the workflows directory stays the same as it is, as well as husky's pre-commit and pre-push configurations

Copilot AI review requested due to automatic review settings February 5, 2026 12:45
@Zaiidmo
Copy link
Contributor

Zaiidmo commented Feb 5, 2026

Unaccepted changes in the operations files !
#Closing the PR

@Zaiidmo Zaiidmo closed this Feb 5, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 87 out of 98 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

useT: () => (key: string, vars?: any) => (typeof key === 'string' ? key : 't'),
}));

vi.mock('react-router', () => ({
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mock uses 'react-router' but the component now imports from 'react-router-dom'. Update the mock to match the actual import source.

Suggested change
vi.mock('react-router', () => ({
vi.mock('react-router-dom', () => ({

Copilot uses AI. Check for mistakes.
Comment on lines +47 to +48

export default undefined;
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exporting undefined as default is unnecessary and confusing. Since the file exports a named function, remove the default export entirely.

Suggested change
export default undefined;

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants