226 lines
8.7 KiB
Markdown
226 lines
8.7 KiB
Markdown
# Enhancement Plan
|
||
|
||
This document identifies current gaps and improvement opportunities in the CRUD flow. Items are grouped by priority and annotated with implementation effort.
|
||
|
||
---
|
||
|
||
## Summary
|
||
|
||
The current CRUD pattern is clean, composable, and type-safe. Most issues are edge cases or developer-experience refinements rather than critical bugs. The highest-priority items are the ones marked **High**.
|
||
|
||
---
|
||
|
||
## High Priority
|
||
|
||
### 1. `useAuthApi` — Missing Memoization
|
||
|
||
**File:** `shared/useApi.ts`
|
||
|
||
**Problem:** `createApi()` is called on every render. Every component that calls `useAuthApi()` creates new `ApiClient` instances per render cycle. This can cause unintended React Query cache misses or stale closures in edge cases.
|
||
|
||
**Fix:**
|
||
|
||
```ts
|
||
// shared/useApi.ts
|
||
import { useMemo } from "react"
|
||
|
||
export const useAuthApi = () => {
|
||
const token = useAuthStore(s => s.token)
|
||
return useMemo(
|
||
() => createApi({ headers: token ? { Authorization: `Bearer ${token}` } : undefined }),
|
||
[token],
|
||
)
|
||
}
|
||
```
|
||
|
||
**Effort:** XS (2 lines) — no API changes required.
|
||
|
||
---
|
||
|
||
### 2. Edit Mode — No Server Re-fetch on Dialog Open
|
||
|
||
**File:** `shared/hooks/use-resource-form.ts`, `modules/customers/customer-form.tsx`
|
||
|
||
**Problem:** When the "Edit" button is clicked, `useResourcePage` passes `selectedItem` (the in-memory table row) as `initialData`. The form is pre-populated from this snapshot. However:
|
||
|
||
- The snapshot may be stale if the item was modified elsewhere.
|
||
- On page refresh with `?dialog=true&resourceId=5` in the URL, `selectedItem` is `null` (not hydrated) → the form opens empty.
|
||
|
||
**Fix:** Add an `initialize` function to each feature form that fetches the full resource by ID:
|
||
|
||
```ts
|
||
// In CustomerForm (use-resource-form options)
|
||
initialize: (id) => api.customers.show(id), // requires CrudClient.show()
|
||
queryKey: [CUSTOMER_ROUTES.BY_ID, resourceId],
|
||
|
||
// In CrudClient (packages/api/src/infra/crud-client.ts)
|
||
async show(id: string) {
|
||
return this.get(this.byIdRoute, { params: { id } } as never)
|
||
}
|
||
```
|
||
|
||
**Effort:** S — needs `CrudClient.show()` added and each form updated to pass `initialize`.
|
||
|
||
---
|
||
|
||
### 3. `FormDialog` — Single Dialog Limitation
|
||
|
||
**File:** `shared/components/form-dialog.tsx`
|
||
|
||
**Problem:** Dialog state is keyed to fixed URL params `dialog` and `resourceId`. If two independent `FormDialog` instances are on the same page (e.g., a main resource form and a nested "Add Customer" side panel), they share the same URL params and will conflict.
|
||
|
||
**Fix:** Accept a configurable `paramKey` prop:
|
||
|
||
```ts
|
||
export const createFormDialogParams = (key: string) => ({
|
||
[`${key}_dialog`]: parseAsBoolean.withDefault(false),
|
||
[`${key}_resourceId`]: parseAsString,
|
||
})
|
||
```
|
||
|
||
**Effort:** S — requires updating `FormDialog`, `useFormDialog`, and `useResourcePage` to accept a `paramKey`.
|
||
|
||
---
|
||
|
||
## Medium Priority
|
||
|
||
### 4. No Global Search / Filter Support
|
||
|
||
**File:** `shared/data-view/table-view/use-data-table-query.ts`, `shared/data-view/table-view/search-params.ts`
|
||
|
||
**Problem:** `dataTableSearchParams` only supports `page`, `per_page`, `sort_by`, `sort_order`. There is no standard way to add resource-specific filters (e.g., search by name, filter by status).
|
||
|
||
**Proposed:** Add an optional `filters` object to `useDataTableQuery` that maps to additional query params:
|
||
|
||
```ts
|
||
useDataTableQuery({
|
||
queryKey: ["customers"],
|
||
client,
|
||
filters: {
|
||
search: parseAsString,
|
||
status: parseAsStringEnum(["active", "inactive"] as const),
|
||
},
|
||
})
|
||
// → adds ?search=&status= params and includes them in client.list()
|
||
```
|
||
|
||
**Effort:** M — requires a design decision and updates to `use-data-table-query`, `data-table.tsx`, and `resource-page`.
|
||
|
||
---
|
||
|
||
### 5. Grid View — Not Implemented
|
||
|
||
**File:** `shared/data-view/grid-view/` (empty directory)
|
||
|
||
**Problem:** The `grid-view` folder was scaffolded but never implemented. The data-view layer is clearly designed to support multiple views (table/grid), but no toggle exists.
|
||
|
||
**Proposed:**
|
||
- Implement a `GridView` component that accepts the same `DataViewProps` as `DataTable`.
|
||
- Add a view toggle (Table | Grid) in the `ResourcePage` header or `DashboardHeader`.
|
||
- Persist the selected view in a URL param (`?view=grid`).
|
||
|
||
**Effort:** M–L depending on grid card design requirements.
|
||
|
||
---
|
||
|
||
### 6. `useMutation` Error Handling — Not Reusable
|
||
|
||
**File:** `modules/customers/customer-form.tsx` (and all other feature forms)
|
||
|
||
**Problem:** The pattern of mapping `ApiError.validationErrors` to `form.setError` is duplicated in every form's `onError` handler.
|
||
|
||
**Fix:** Extract a `useFormMutation` hook:
|
||
|
||
```ts
|
||
// shared/hooks/use-form-mutation.ts
|
||
export function useFormMutation<TValues extends FieldValues, TResponse = unknown>(
|
||
form: UseFormReturn<TValues>,
|
||
options: UseMutationOptions<TResponse, Error, TValues>,
|
||
) {
|
||
return useMutation({
|
||
...options,
|
||
onError: (err, vars, ctx) => {
|
||
if (err instanceof ApiError && err.validationErrors) {
|
||
Object.entries(err.validationErrors).forEach(([field, msgs]) => {
|
||
form.setError(field as keyof TValues as any, { message: msgs[0] })
|
||
})
|
||
}
|
||
options.onError?.(err, vars, ctx)
|
||
},
|
||
})
|
||
}
|
||
```
|
||
|
||
**Effort:** XS — purely additive. Existing forms can be migrated incrementally.
|
||
|
||
---
|
||
|
||
### 7. `CUSTOMER_CREATED_EVENT` — Unused Custom Event
|
||
|
||
**File:** `modules/customers/customer-form.tsx`
|
||
|
||
**Problem:** `CustomerForm` dispatches `window.dispatchEvent(new CustomEvent("customer:created"))` on success, but nothing in the codebase listens to this event. Cache invalidation is already handled via `onSuccess` → `invalidateQuery()`. The event dispatch is dead code.
|
||
|
||
**Fix:** Remove the event dispatch from `CustomerForm` (and the `CUSTOMER_CREATED_EVENT` export) unless there is a known future use case, such as notifying a sibling component outside the React tree.
|
||
|
||
**Effort:** XS.
|
||
|
||
---
|
||
|
||
### 8. Pagination Meta Split — Inconsistency
|
||
|
||
**File:** `shared/data-view/table-view/use-data-table-query.ts`
|
||
|
||
**Problem:** `useDataTableQuery` returns `pagination` with `pageCount: 1, total: 0` as placeholders. The real values come from `data.meta` and are calculated inside `ResourcePage.tsx`. This means:
|
||
|
||
- `useResourcePage` consumers who render the table directly (outside `ResourcePage`) need to duplicate the `pageCount`/`total` derivation.
|
||
- The `pagination` object returned by the hook is misleading until data loads.
|
||
|
||
**Fix:** Either move the meta derivation inside `useDataTableQuery` (requiring it to accept a response shape hint), or document this as an intentional split and annotate it.
|
||
|
||
**Effort:** XS–S.
|
||
|
||
---
|
||
|
||
## Low Priority / Nice to Have
|
||
|
||
### 9. Row Selection for Bulk Actions
|
||
|
||
There is no row selection or bulk-delete support. TanStack Table supports `rowSelection` state natively. Adding a checkbox column and a "Delete selected" toolbar would benefit resource-heavy pages.
|
||
|
||
### 10. Error Boundary Around Table and Form
|
||
|
||
If a render error occurs inside `DataTable` or a feature form, it will bubble up and crash the whole page. Wrapping with `<ErrorBoundary>` (e.g., via `react-error-boundary`) would improve resilience.
|
||
|
||
### 11. `ConfirmDialog` — Not Enforced in Layout
|
||
|
||
**Problem:** `<ConfirmDialog />` must be manually mounted in `app/(authenticated)/layout.tsx`. There is no lint rule or runtime warning if it is missing. If a developer forgets to add it to a new layout, `confirm()` calls will silently resolve to `false` (no dialog shown, deletion blocked).
|
||
|
||
**Fix:** Add a development-mode warning inside the `confirm()` function if the store's `resolve` is never set after a timeout.
|
||
|
||
### 12. Column Visibility / Hide
|
||
|
||
`ColumnHeader` has a "Hide" dropdown menu item (via `column.toggleVisibility(false)`), but there is no global "Show columns" control to restore hidden columns. Either remove the hide option or add a column visibility popup to `DataTable`.
|
||
|
||
### 13. `dataTableSearchParamsCache` — Imported but Unused in App Router
|
||
|
||
`dataTableSearchParamsCache` is exported but the pages use `"use client"` throughout. If server components are introduced for any list page, the cache needs wiring in the layout via `nuqs`'s `SearchParamsProvider`.
|
||
|
||
---
|
||
|
||
## Checklist
|
||
|
||
- [x] #1 — Memoize `useAuthApi`
|
||
- [x] #2 — Add `CrudClient.show()` and wire `initialize` in feature forms
|
||
- [x] #3 — Make `FormDialog` param key configurable
|
||
- [ ] #4 — Design and implement filter/search param support in `useDataTableQuery`
|
||
- [ ] #5 — Implement `GridView` and view toggle
|
||
- [x] #6 — Extract `useFormMutation` hook
|
||
- [x] #7 — Remove unused `CUSTOMER_CREATED_EVENT`
|
||
- [x] #8 — Move pagination meta derivation into `useDataTableQuery`
|
||
- [ ] #9 — Row selection + bulk actions
|
||
- [ ] #10 — Error boundaries
|
||
- [x] #11 — Dev-mode warning for missing `ConfirmDialog`
|
||
- [ ] #12 — Column visibility restore control
|
||
- [ ] #13 — Wire `SearchParamsProvider` if server components are adopted
|