bermudalamb/redefined-designs
1
0
Fork 0
Code Issues 2 Pull Requests 1 Actions Packages Projects 3 Releases Wiki Activity

Import convention and avoidable any are inconsistent across the codebase #65

New Issue
Closed
opened 2026-08-19 11:07:00 -05:00 by bermudalamb · 1 comment
bermudalamb commented 2026-08-19 11:07:00 -05:00
Owner
Copy Link
Copy Source

Found during a Microsoft/React/SonarQube best-practices review. Lower severity than the sibling issues — maintainability rather than correctness — but both are cheap to settle and currently drift with every new file.

1. Two antd import styles, in roughly equal measure

The documented convention is deep imports from antd/es/*. Nine files use the barrel instead:

admin/Admin.tsx, admin/Customers.tsx, admin/Settings.tsx, App.tsx, cart/Cart.tsx, components/ItemCard.tsx, customer/Account.tsx, customer/PrivacyPolicy.tsx, main.tsx

Newer files (FilterDrawer.tsx, AuthForm.tsx, AuthRouteModal.tsx, ForgotPassword.tsx, ResetPassword.tsx) follow the deep-import convention, so the split runs roughly along the age of the file and will keep widening.

Worth being accurate about the stakes: with antd v5 and Vite both styles resolve to the same ES modules, so this is not the tree-shaking problem it would be under v4, and it is not the antd/lib mistake that loads a second React context and breaks ConfigProvider. The real cost is that there is a documented convention which half the codebase does not follow, so nobody can tell whether a given file is deliberate or just old.

The decision needed is simply which way to standardise, then apply it — not investigation.

2. Seven any annotations where a real type is available

Location Current Available
backend/src/routes/cartCheckout.ts:40, 66, 147 client: any PoolClient from pg — already imported in admin.ts
backend/src/routes/customers.ts:35 publicCustomer(c: any) a Customer row type
frontend/src/cart/Cart.tsx:110, 112, frontend/src/paypal.ts:4 (window as any).paypal a declared interface for the injected SDK

The PoolClient ones are the most worth fixing: those three functions run inside transactions, and any removes exactly the type checking that would catch a client/pool mix-up — which in this codebase means a query silently running outside the transaction it was meant to be part of.

The window.paypal casts are the most defensible, since the SDK is injected at runtime by a script tag. A small declare global interface would still beat a cast repeated in three places.

strict: true is already on, so these anys are the deliberate holes in an otherwise checked codebase rather than the default state.

Suggested handling

Both are mechanical once decided, and both are the kind of thing that stays consistent only if enforced — an ESLint rule (no-explicit-any, plus a no-restricted-imports rule for the antd barrel) would settle them permanently. That makes this a natural follow-on to the ESLint issue rather than a separate cleanup that decays again.

Severity

Low. No defect today.

Found during a Microsoft/React/SonarQube best-practices review. Lower severity than the sibling issues — maintainability rather than correctness — but both are cheap to settle and currently drift with every new file. ## 1. Two antd import styles, in roughly equal measure The documented convention is deep imports from `antd/es/*`. Nine files use the barrel instead: `admin/Admin.tsx`, `admin/Customers.tsx`, `admin/Settings.tsx`, `App.tsx`, `cart/Cart.tsx`, `components/ItemCard.tsx`, `customer/Account.tsx`, `customer/PrivacyPolicy.tsx`, `main.tsx` Newer files (`FilterDrawer.tsx`, `AuthForm.tsx`, `AuthRouteModal.tsx`, `ForgotPassword.tsx`, `ResetPassword.tsx`) follow the deep-import convention, so the split runs roughly along the age of the file and will keep widening. Worth being accurate about the stakes: with antd v5 and Vite both styles resolve to the same ES modules, so this is **not** the tree-shaking problem it would be under v4, and it is not the `antd/lib` mistake that loads a second React context and breaks `ConfigProvider`. The real cost is that there is a documented convention which half the codebase does not follow, so nobody can tell whether a given file is deliberate or just old. The decision needed is simply which way to standardise, then apply it — not investigation. ## 2. Seven `any` annotations where a real type is available | Location | Current | Available | | --- | --- | --- | | `backend/src/routes/cartCheckout.ts:40, 66, 147` | `client: any` | `PoolClient` from `pg` — already imported in `admin.ts` | | `backend/src/routes/customers.ts:35` | `publicCustomer(c: any)` | a `Customer` row type | | `frontend/src/cart/Cart.tsx:110, 112`, `frontend/src/paypal.ts:4` | `(window as any).paypal` | a declared interface for the injected SDK | The `PoolClient` ones are the most worth fixing: those three functions run inside transactions, and `any` removes exactly the type checking that would catch a client/pool mix-up — which in this codebase means a query silently running outside the transaction it was meant to be part of. The `window.paypal` casts are the most defensible, since the SDK is injected at runtime by a script tag. A small `declare global` interface would still beat a cast repeated in three places. `strict: true` is already on, so these `any`s are the deliberate holes in an otherwise checked codebase rather than the default state. ## Suggested handling Both are mechanical once decided, and both are the kind of thing that stays consistent only if enforced — an ESLint rule (`no-explicit-any`, plus a `no-restricted-imports` rule for the antd barrel) would settle them permanently. That makes this a natural follow-on to the ESLint issue rather than a separate cleanup that decays again. ## Severity Low. No defect today.
bermudalamb added this to the Code Quality and Hardening project 2026-08-19 11:38:36 -05:00
bermudalamb commented 2026-08-21 15:41:30 -05:00
Author
Owner
Copy Link
Copy Source

Scope confirmed: this issue owns the type-checked gate too, not just its own two lists

#60 deferred recommendedTypeChecked and the no-unsafe-* family here on the grounds that they trace to pool.query() and fetch().json() returning any, and that typing those is the content of this issue. That is correct, and it makes this considerably larger than the two inventories below suggest. Confirmed as in scope.

Measured, not estimated

This issue's own two lists are unchanged since it was written — still exactly 9 barrel-import files and 7 any sites.

#60's "~325" is now measured per file, by running recommendedTypeChecked against the current tree:

no-unsafe-* findings Files
Backend 259 16
Frontend 73 11

The frontend's raw total is 102, but 25 of those are no-misused-promises on the antd onClick={async …} pattern, which #60 already dispositioned with checksVoidReturn: { attributes: false }. They are not new work.

Both are concentrated rather than spread. cartCheckout.ts (83) and customers.ts (54) are more than half the backend; api.ts (17) plus the four *Api.ts modules (27) are most of the frontend.

Four stages, each its own branch and review

Not a narrowing of scope — all of it lands here — but 331 findings plus an import migration in one commit is unreviewable, and each stage leaves the tree working and verifiable.

1. This issue's original body. Nine barrel imports to antd/es/*; the seven any sites fixed — PoolClient for the three transaction helpers, a row type for publicCustomer, and a declare global interface for the injected PayPal SDK replacing three (window as any) casts. Independent of the rest.

2. The frontend boundary. One generic helper so the cast happens once rather than implicitly at every call site:

async function json<T>(res: Response): Promise<T> {
  return (await res.json()) as T;
}

These functions already declare Promise<Item[]> and similar. This makes those declarations true rather than aspirational, and clears the API layer's ~44 findings along with most of the ~29 in components that follow from them.

3. The backend boundary — the large one. Row interfaces describing query results rather than tables, because the real shapes are joined and computed: ADMIN_ITEM_SELECT returns i.* plus category_name, images and tags. itemSelect.ts already centralises two such shapes and is the pattern to follow. pool.query<Row>() then types rows, and the unsafe member accesses go with it.

4. Flip the gate. recommendedTypeChecked on in both workspaces with no-unsafe-* at error — honest only once 2 and 3 are green.

Where this could go wrong

Stage 3 is the risk. If typing cartCheckout.ts turns out to need restructuring rather than annotating, that is a different change from the one agreed here, and it stops and comes back to the issue rather than quietly widening.

Counts get re-measured after each stage, so progress is observed rather than asserted.

## Scope confirmed: this issue owns the type-checked gate too, not just its own two lists #60 deferred `recommendedTypeChecked` and the `no-unsafe-*` family here on the grounds that they trace to `pool.query()` and `fetch().json()` returning `any`, and that typing those is the content of this issue. That is correct, and it makes this considerably larger than the two inventories below suggest. Confirmed as in scope. ## Measured, not estimated This issue's own two lists are unchanged since it was written — still exactly **9 barrel-import files** and **7 `any` sites**. #60's "~325" is now measured per file, by running `recommendedTypeChecked` against the current tree: | | `no-unsafe-*` findings | Files | | --- | --- | --- | | Backend | **259** | 16 | | Frontend | **73** | 11 | The frontend's raw total is 102, but 25 of those are `no-misused-promises` on the antd `onClick={async …}` pattern, which #60 already dispositioned with `checksVoidReturn: { attributes: false }`. They are not new work. Both are concentrated rather than spread. `cartCheckout.ts` (83) and `customers.ts` (54) are more than half the backend; `api.ts` (17) plus the four `*Api.ts` modules (27) are most of the frontend. ## Four stages, each its own branch and review Not a narrowing of scope — all of it lands here — but 331 findings plus an import migration in one commit is unreviewable, and each stage leaves the tree working and verifiable. **1. This issue's original body.** Nine barrel imports to `antd/es/*`; the seven `any` sites fixed — `PoolClient` for the three transaction helpers, a row type for `publicCustomer`, and a `declare global` interface for the injected PayPal SDK replacing three `(window as any)` casts. Independent of the rest. **2. The frontend boundary.** One generic helper so the cast happens once rather than implicitly at every call site: ```ts async function json<T>(res: Response): Promise<T> { return (await res.json()) as T; } ``` These functions already declare `Promise<Item[]>` and similar. This makes those declarations true rather than aspirational, and clears the API layer's ~44 findings along with most of the ~29 in components that follow from them. **3. The backend boundary** — the large one. Row interfaces describing **query results rather than tables**, because the real shapes are joined and computed: `ADMIN_ITEM_SELECT` returns `i.*` plus `category_name`, `images` and `tags`. `itemSelect.ts` already centralises two such shapes and is the pattern to follow. `pool.query<Row>()` then types `rows`, and the unsafe member accesses go with it. **4. Flip the gate.** `recommendedTypeChecked` on in both workspaces with `no-unsafe-*` at error — honest only once 2 and 3 are green. ## Where this could go wrong Stage 3 is the risk. If typing `cartCheckout.ts` turns out to need restructuring rather than annotating, that is a different change from the one agreed here, and it stops and comes back to the issue rather than quietly widening. Counts get re-measured after each stage, so progress is observed rather than asserted.
bermudalamb moved this to Review in Code Quality and Hardening on 2026-08-21 16:05:03 -05:00
bermudalamb self-assigned this 2026-08-21 16:10:10 -05:00
bermudalamb added reference feature/65-imports-and-any 2026-08-21 16:14:54 -05:00
bermudalamb moved this to Ready for Release in Code Quality and Hardening on 2026-08-21 16:47:05 -05:00
Sign in to join this conversation.
feature/65-imports-and-any
Branches Tags
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Code Quality and Hardening
Ready for Release
Assignees
Clear assignees
calamb (Chelsea Lamb) bermudalamb (Thom Lamb)
No Assignees bermudalamb
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: bermudalamb/redefined-designs#65