Skip to main content

Core frontend (Next.js)

Type: ReferenceCreated: Team: Platform
draft

About this prompt

This is the exact review prompt the AI reviewer runs for core frontend pull requests, applied on top of the shared review rules. Skim it before you raise a PR — most of what it flags is something you can catch and fix yourself first.

# Skapp Frontend Review Guidelines

> Next.js 14 (Pages Router) + React 18 + TypeScript (strict) + MUI 6 + Tailwind 4 + Zustand + React Query 5 + Formik/Yup + i18next

## Architecture

This is a modular monorepo frontend with community/enterprise mode separation. Enterprise features live in git submodules at `src/enterprise/` (source), `pages/enterprise/` (pages), and `pages/api/enterprise/` (API routes), with fallback stubs at `src/fallback/`. Mode is toggled via `NEXT_PUBLIC_MODE` env var, with route rewrites in `next.config.mjs`.

### Module structure
Each feature module (under `src/community/` and `src/enterprise/`) follows:
```
module/
api/ → React Query hooks + Axios calls
components/ → atoms / molecules / organisms / templates
store/ → Zustand slices
utils/ → Module-specific helpers
types/ → TypeScript types and enums
```

### Key conventions
- **Path aliases**: `~community/*`, `~enterprise/*`, `~styles/*`, `~public/*`, `~i18n`, `~middleware`, `~firebase` — never use deep relative imports across modules
- **Component files**: PascalCase with barrel `index.tsx` exports

---

## What to flag

### React Query misuse
- Missing or incorrect query key factories — keys must use the established `get*QueryKeys` pattern (e.g., `getAttendanceQueryKeys.employeeStatus()`)
- Forgetting `queryClient.invalidateQueries()` after mutations that affect cached data
- Using `useQuery` without an `enabled` flag when the query depends on runtime state
- Stale closures in `onSuccess`/`onError` — prefer query key invalidation over manual cache updates
- Side effects in `useEffect` reacting to query `data` instead of handling in `onSuccess`

### Zustand store issues
- State that belongs in React Query server cache being duplicated in Zustand
- Missing devtools middleware on new stores (all stores use `devtools()` wrapper)
- Slices that mutate state outside `set()` — Zustand requires immutable updates via the setter
- Adding global state for data that should be local component state

### Type safety
- `any` types — strict mode is enabled, every type must be explicit
- Missing or incomplete interface definitions for component props
- Using type assertions (`as`) to silence errors instead of fixing the underlying type
- Enum values that don't match backend contracts (check against backend `type/` enums)

### API layer
- Direct `axios` calls outside the established service pattern — all API calls go through `authFetch` or `authFetchV2` interceptors
- Missing error handling on mutations — must show user feedback (toast) on failure
- Hardcoded API URLs instead of using endpoint constant files (`*EndPoints.ts`)
- Not using the correct API version (`v1` vs `v2`) for the endpoint

### Component patterns
- Class components — this codebase is 100% functional components with hooks
- Props drilling through more than 2 levels when a Zustand store or context exists for that data
- Missing `key` prop on list items or using array index as key on reorderable lists
- Components doing data fetching AND rendering — separate into container/presenter or use React Query hooks directly
- Large components (>200 lines) that should be decomposed into atoms/molecules

### Styling
- Mixing styling approaches inconsistently — MUI `sx` prop for MUI components, Tailwind classes for layout/custom elements
- Hardcoded color values instead of using theme tokens (`theme.palette.*` for MUI, Tailwind theme vars)
- Inline styles that should be in colocated `styles.ts` files
- Missing responsive handling — check MUI breakpoints or Tailwind responsive prefixes

### Forms (Formik + Yup)
- Form state managed with `useState` instead of Formik — all forms use Formik
- Missing Yup validation schemas or incomplete validation rules
- Not using `@Valid` backend validation as the source of truth for field constraints
- Custom validation logic that duplicates what Yup already provides

### i18n
- Hardcoded user-facing strings — all text must go through `useTranslator` / `t()` from i18next
- Translation keys that don't follow the namespace convention (`module.section.key`)
- Missing translations for error messages, tooltips, and accessibility labels

### Security & auth
- Route access not gated by role — check against `AdminTypes`, `ManagerTypes`, `EmployeeTypes` enums
- Sensitive data stored in localStorage instead of secure cookies
- Missing authentication checks on pages that require login
- Tenant ID (`X-Tenant-ID`) not being sent for enterprise mode requests

### Performance
- Missing `useMemo` / `useCallback` on expensive computations or callbacks passed to child components
- Large bundle imports (e.g., importing entire MUI icon set instead of individual icons)
- React Query fetches without proper `staleTime` causing unnecessary refetches
- Missing loading/skeleton states during async operations

### Testing
- New components without corresponding `.test.tsx` files
- Tests that mock implementation details instead of testing behavior
- Missing `MockTheme` wrapper in component tests (MUI theme is required)
- Tests that don't use `@testing-library/react` patterns (prefer `screen.getByRole` over `getByTestId`)

---

## What NOT to flag (false positives)

- MUI + Tailwind coexistence — this is intentional, not a smell
- `useEffect` for syncing React Query data to Zustand stores — this is the established pattern
- Enterprise/community conditional rendering — this is the architecture, not complexity
- Prettier/ESLint fixable issues (import order, formatting) — these are caught by pre-commit hooks

---

## Checklist

Verify each item for every changed file:

1. **Type safety**: Any `any`, `as` assertion, or missing prop type? Every variable, parameter, and return value must have an explicit type
2. **Null/undefined safety**: Optional chaining (`?.`) where needed? Check API response shapes — can fields be `undefined` or `null` at runtime?
3. **React Query correctness**: Correct query key factory used? `invalidateQueries` called after mutations that affect cached data? `enabled` flag set when query depends on runtime state?
4. **Error handling**: Every mutation has `onError` with user-visible feedback (toast)? Loading and error states rendered in the UI?
5. **Behavioral preservation in refactors**: When hooks or components are reorganized — same dependencies in `useEffect`/`useMemo`? Same query keys? Same invalidation patterns? Same error handling?
6. **Dead code**: Unused imports, unreachable branches, state variables that are set but never read, effects with empty dependency arrays that should have deps
7. **i18n completeness**: Any new user-facing string missing `t()` wrapping? Translation keys follow `module.section.key` convention?
8. **Naming conventions**: Components in PascalCase? Hooks prefixed with `use`? Query key factories follow `get*QueryKeys` pattern? Files in correct module directory?
9. **Code duplication**: 2+ components with near-identical markup → extract shared component. 2+ hooks with same logic → extract custom hook
10. **Security**: New pages gated by correct role enum? Sensitive data not in localStorage? Tenant ID sent for enterprise requests?