From f32913ef514540aaa92cda3cfe1a5c91511a1a49 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Mon, 24 Aug 2026 15:25:09 -0500 Subject: [PATCH] refactor: turn on noUncheckedIndexedAccess in both workspaces (#101) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Indexing an array now yields `T | undefined`, which is what it always did — the compiler simply says so. Enabled in backend/tsconfig.json and frontend/tsconfig.json, and in tsconfig.sonar.json alongside it so the drift guard stays satisfied. The sequencing this issue insisted on was right, and the numbers show why. Before #159 typed the query results, `rows[0]` was `any` and `any` indexes to `any`: the flag would have found close to nothing and the count would have changed completely afterwards. With the rows typed it finds 57 — 50 in the backend, 4 in the frontend, 3 in the Playwright suite — and they fall into three kinds. Thirty are `rows[0]` after a `rows.length` guard. TypeScript cannot connect the two, and rewriting them as `const [row] = rows; if (!row) …` makes the guard and the use the same check, which is better code independently of the flag. Ten are rows a statement guarantees — `INSERT … RETURNING`, or a lookup for an id the session middleware has already matched. These get `requireRow(rows, what)`, a new helper in db.ts that throws naming the query. A thrown error rather than a non-null assertion: if the assumption is ever wrong, an assertion hands `undefined` to the next line and fails somewhere unrelated, whereas this fails at the query and says which. asyncRoute turns it into a 500, which is the right answer for "the database did not do what the statement says it does". It also states the assumption once instead of ten times. The rest is ordinary indexing the compiler cannot prove: a regex capture group that the pattern guarantees, `split('+')[0]`, a modulo kept in range, `hasOwnProperty` failing to narrow an index signature, and Express typing route params as an index signature so `req.params.itemId` is `string | undefined` on a route that cannot match without it. One correction to this issue's premise, which matters for what it was expected to find. The body says "in a handful it does not guard at all", and the unguarded-500 risk it describes was not found. Every `rows[0]` either sits behind a length check or behind a statement that guarantees a row. What the flag actually bought was the ten places where that guarantee was real but unstated, and those now say so. Two changes worth calling out because they are not mechanical. TAG_COLORS is typed `[string, ...string[]]` in both copies rather than `as const` — the first attempt used `as const`, which narrowed the elements to literals and broke adminTags, so the annotation keeps `string` while telling the compiler index 0 exists. And the filter drawer's slider falls back to the bounds it was given rather than to null, because null there reads as "no filter" and would widen the results rather than leave them unchanged. Test files needed changes too, since ts-jest compiles them against the same config: a regex destructure in the compose guard, and ten `mock.calls[0][0]` reads where the surrounding assertions already establish the call happened. Verified: tsc clean over backend, frontend src and the Playwright suite; unit 254/254; integration 238/238; frontend build clean; lint unchanged in both workspaces. Closes #101 --- backend/src/db.ts | 25 +++++++++++++ backend/src/emailTemplates.ts | 16 ++++++-- backend/src/mailer.ts | 4 +- backend/src/middleware/customerAuth.ts | 3 +- backend/src/routes/admin.ts | 13 ++++--- backend/src/routes/adminCategories.ts | 4 +- backend/src/routes/cart.ts | 22 +++++++---- backend/src/routes/cartCheckout.ts | 14 ++++--- backend/src/routes/customers.ts | 37 ++++++++++--------- backend/src/routes/public.ts | 4 +- backend/src/utils.ts | 11 +++++- .../clientErrors.integration.test.ts | 16 ++++---- .../resendVerification.integration.test.ts | 2 +- backend/tests/unit/adminGate.test.ts | 2 +- backend/tests/unit/composeEnvironment.test.ts | 7 +++- backend/tsconfig.json | 5 +++ frontend/src/admin/Tags.tsx | 2 +- frontend/src/components/FilterDrawer.tsx | 10 ++++- frontend/tests/e2e/admin-theme.spec.ts | 4 +- frontend/tsconfig.json | 3 ++ frontend/tsconfig.sonar.json | 3 ++ 21 files changed, 144 insertions(+), 63 deletions(-) diff --git a/backend/src/db.ts b/backend/src/db.ts index 54461c5..1bf095b 100755 --- a/backend/src/db.ts +++ b/backend/src/db.ts @@ -7,3 +7,28 @@ export const pool = new Pool({ password: process.env.PGPASSWORD, database: process.env.PGDATABASE }); + +/** + * The single row a query is guaranteed to have returned. + * + * For `INSERT ... RETURNING` and `UPDATE ... WHERE id = $1 RETURNING` after the + * row's existence has already been established: Postgres returns exactly one + * row, so there is nothing to branch on, but `noUncheckedIndexedAccess` is right + * that `rows[0]` is `T | undefined` and the compiler cannot know better. + * + * A thrown error rather than a non-null assertion. If the assumption is ever + * wrong the assertion would hand `undefined` to the next line and fail somewhere + * unrelated, whereas this fails here and says which query. `asyncRoute` turns it + * into a 500, which is the right answer for "the database did not do what the + * statement says it does". + * + * Reads that legitimately might find nothing do not use this — they destructure + * and branch, so the check and the use are the same thing. + */ +export function requireRow(rows: T[], what: string): T { + const row = rows[0]; + if (!row) { + throw new Error(`expected ${what} to return a row, got none`); + } + return row; +} diff --git a/backend/src/emailTemplates.ts b/backend/src/emailTemplates.ts index 404498f..010c674 100644 --- a/backend/src/emailTemplates.ts +++ b/backend/src/emailTemplates.ts @@ -179,15 +179,23 @@ const PLACEHOLDER = /\{\{\s*(\w+)\s*\}\}/g; export function missingPlaceholders(key: TemplateKey, body: string): string[] { const present = new Set(); for (const match of body.matchAll(PLACEHOLDER)) { - present.add(match[1]); + // PLACEHOLDER has exactly one capture group, so a match always has [1] — + // but a RegExpMatchArray cannot say so, hence the guard rather than an + // assertion. A match without it would be a change to the pattern. + const name = match[1]; + if (name) present.add(name); } return TEMPLATES[key].required.filter((name) => !present.has(name)); } function substitute(text: string, values: Record): string { - return text.replace(PLACEHOLDER, (whole, name: string) => - Object.prototype.hasOwnProperty.call(values, name) ? values[name] : whole - ); + return text.replace(PLACEHOLDER, (whole, name: string) => { + // hasOwnProperty does not narrow an index signature, so the lookup is done + // once and tested. Checking the value also treats an explicitly-undefined + // entry the same as a missing one, which is what the caller means. + const value = values[name]; + return value === undefined ? whole : value; + }); } /** diff --git a/backend/src/mailer.ts b/backend/src/mailer.ts index 31a40a9..d40bd29 100755 --- a/backend/src/mailer.ts +++ b/backend/src/mailer.ts @@ -31,7 +31,9 @@ function parseAddress(address: string): ParsedAddress | null { } const localWithSuffix = trimmed.slice(0, at); return { - local: localWithSuffix.split('+')[0], + // split always yields at least one element, so this cannot actually be + // undefined — but String.split's type cannot express that. + local: localWithSuffix.split('+')[0] ?? localWithSuffix, domain: trimmed.slice(at + 1) }; } diff --git a/backend/src/middleware/customerAuth.ts b/backend/src/middleware/customerAuth.ts index d514acc..793a0f4 100755 --- a/backend/src/middleware/customerAuth.ts +++ b/backend/src/middleware/customerAuth.ts @@ -28,7 +28,8 @@ export async function attachCustomer(req: Request, _res: Response, next: NextFun WHERE s.token = $1 AND s.expires_at > now() AND c.disabled_at IS NULL`, [token] ); - if (rows.length) req.customerId = rows[0].customer_id; + const [session] = rows; + if (session) req.customerId = session.customer_id; next(); } diff --git a/backend/src/routes/admin.ts b/backend/src/routes/admin.ts index df70e65..a259678 100755 --- a/backend/src/routes/admin.ts +++ b/backend/src/routes/admin.ts @@ -3,7 +3,7 @@ import multer from 'multer'; import { promises as fs } from 'fs'; import { randomUUID } from 'crypto'; import { PoolClient } from 'pg'; -import { pool } from '../db'; +import { pool, requireRow } from '../db'; import { ADMIN_ITEM_SELECT, AdminItemRow, ItemRecord } from '../itemSelect'; import { ItemStatus } from '../types'; import { asyncRoute } from '../asyncRoute'; @@ -270,11 +270,11 @@ router.post('/items', uploadImages, asyncRoute(async (req: Request, res: Respons `INSERT INTO items (name, description, price_cents, category_id) VALUES ($1, $2, $3, $4) RETURNING *`, [name, description, Math.round(parseFloat(price) * 100), categoryId ?? null] ); - const item = rows[0]; + const item = requireRow(rows, 'the item INSERT'); for (let i = 0; i < files.length; i++) { await client.query( `INSERT INTO item_images (item_id, image_path, sort_order) VALUES ($1, $2, $3)`, - [item.id, `/uploads/${files[i].filename}`, i] + [item.id, `/uploads/${files[i]?.filename ?? ''}`, i] ); } if (tagNames) { @@ -282,7 +282,7 @@ router.post('/items', uploadImages, asyncRoute(async (req: Request, res: Respons } await client.query('COMMIT'); const { rows: full } = await pool.query(`${ADMIN_ITEM_SELECT} WHERE i.id = $1`, [item.id]); - res.json(full[0]); + res.json(requireRow(full, 'the item just inserted')); } catch (err) { await client.query('ROLLBACK'); console.error(err); @@ -325,7 +325,8 @@ router.put('/items/:id', uploadImages, asyncRoute(async (req: Request, res: Resp `SELECT COALESCE(MAX(sort_order), -1) AS max_sort FROM item_images WHERE item_id = $1`, [req.params.id] ); - let nextSort = existing[0].max_sort + 1; + // COALESCE'd MAX, so the aggregate always returns exactly one row. + let nextSort = requireRow(existing, 'the MAX(sort_order) aggregate').max_sort + 1; for (const file of files) { await client.query( `INSERT INTO item_images (item_id, image_path, sort_order) VALUES ($1, $2, $3)`, @@ -388,7 +389,7 @@ router.post('/items/:id/unpublish', asyncRoute(async (req: Request, res: Respons return res.status(404).json({ error: 'not found' }); } - const status = rows[0].status; + const status = requireRow(rows, 'the item status lookup').status; if (status === 'pending') { return res.status(400).json({ error: 'this item is already pending' }); } diff --git a/backend/src/routes/adminCategories.ts b/backend/src/routes/adminCategories.ts index 9c7ce1f..a8d66da 100644 --- a/backend/src/routes/adminCategories.ts +++ b/backend/src/routes/adminCategories.ts @@ -1,5 +1,5 @@ import { Router, Request, Response } from 'express'; -import { pool } from '../db'; +import { pool, requireRow } from '../db'; import { asyncRoute } from '../asyncRoute'; interface CategoryRow { @@ -205,7 +205,7 @@ router.delete('/:id', asyncRoute(async (req: Request, res: Response) => { // being deleted along with their category. await pool.query(`DELETE FROM categories WHERE id = $1`, [id]); - res.json({ deleted_categories: ids.length, uncategorized_items: affected[0].n }); + res.json({ deleted_categories: ids.length, uncategorized_items: requireRow(affected, 'the affected-items COUNT').n }); })); export default router; diff --git a/backend/src/routes/cart.ts b/backend/src/routes/cart.ts index df7bbee..7d2db98 100644 --- a/backend/src/routes/cart.ts +++ b/backend/src/routes/cart.ts @@ -1,5 +1,5 @@ import { Router, Request, Response } from 'express'; -import { pool } from '../db'; +import { pool, requireRow } from '../db'; import { asyncRoute } from '../asyncRoute'; import { requireCustomer } from '../middleware/customerAuth'; import { getSettings } from '../adminSettings'; @@ -57,13 +57,17 @@ const CART_ITEM_SELECT = ` router.get('/', requireCustomer, asyncRoute(async (req: Request, res: Response) => { const { rows: cartRows } = await pool.query(`SELECT id FROM carts WHERE customer_id = $1`, [req.customerId]); - if (!cartRows.length) return res.json({ items: [] }); - const { rows: items } = await pool.query(CART_ITEM_SELECT, [cartRows[0].id]); + const [cart] = cartRows; + if (!cart) return res.json({ items: [] }); + const { rows: items } = await pool.query(CART_ITEM_SELECT, [cart.id]); res.json({ items }); })); router.post('/items/:itemId', requireCustomer, asyncRoute(async (req: Request, res: Response) => { + // Express types route params as an index signature, so this is + // `string | undefined` even though the route cannot match without it. const itemId = req.params.itemId; + if (!itemId) return res.status(400).json({ error: 'itemId is required' }); const client = await pool.connect(); try { await client.query('BEGIN'); @@ -75,17 +79,18 @@ router.post('/items/:itemId', requireCustomer, asyncRoute(async (req: Request, r return res.status(409).json({ error: 'item is no longer available' }); } - let { rows: cartRows } = await client.query(`SELECT id FROM carts WHERE customer_id = $1`, [req.customerId]); + const { rows: cartRows } = await client.query(`SELECT id FROM carts WHERE customer_id = $1`, [req.customerId]); + const [existingCart] = cartRows; let cartId: number; - if (cartRows.length) { - cartId = cartRows[0].id; + if (existingCart) { + cartId = existingCart.id; await client.query(`UPDATE carts SET updated_at = now() WHERE id = $1`, [cartId]); } else { const { rows: newCart } = await client.query( `INSERT INTO carts (customer_id) VALUES ($1) RETURNING id`, [req.customerId] ); - cartId = newCart[0].id; + cartId = requireRow(newCart, 'the cart INSERT').id; } const { cartExpiryHours } = await getSettings(); @@ -107,7 +112,10 @@ router.post('/items/:itemId', requireCustomer, asyncRoute(async (req: Request, r })); router.delete('/items/:itemId', requireCustomer, asyncRoute(async (req: Request, res: Response) => { + // Express types route params as an index signature, so this is + // `string | undefined` even though the route cannot match without it. const itemId = req.params.itemId; + if (!itemId) return res.status(400).json({ error: 'itemId is required' }); const client = await pool.connect(); try { await client.query('BEGIN'); diff --git a/backend/src/routes/cartCheckout.ts b/backend/src/routes/cartCheckout.ts index 2648d63..b585a29 100644 --- a/backend/src/routes/cartCheckout.ts +++ b/backend/src/routes/cartCheckout.ts @@ -1,6 +1,6 @@ import { Router, Request, Response } from 'express'; import type { PoolClient } from 'pg'; -import { pool } from '../db'; +import { pool, requireRow } from '../db'; import { asyncRoute } from '../asyncRoute'; import { requireCustomer } from '../middleware/customerAuth'; import { notifyFavoritersOfSale } from '../favoriteAlerts'; @@ -70,8 +70,9 @@ interface LockedCart { // and returns { cartId, items: [{id, name, price_cents}], totalCents }. async function loadLockedCart(client: PoolClient, customerId: number): Promise { const { rows: cartRows } = await client.query(`SELECT id FROM carts WHERE customer_id = $1`, [customerId]); - if (!cartRows.length) return null; - const cartId = cartRows[0].id; + const [cart] = cartRows; + if (!cart) return null; + const cartId = cart.id; const { rows: items } = await client.query( `SELECT i.id, i.name, i.price_cents FROM cart_items ci @@ -114,7 +115,7 @@ async function openCheckout( VALUES ($1, $2, $3, $4, $5, 'pending') RETURNING id`, [customerId, shippingAddressId, processor, processorOrderId, cart.totalCents] ); - const checkoutId = checkoutRows[0].id; + const checkoutId = requireRow(checkoutRows, 'the checkout INSERT').id; for (const it of cart.items) { await client.query( `INSERT INTO checkout_items (checkout_id, item_id, price_cents) VALUES ($1, $2, $3)`, @@ -221,7 +222,7 @@ router.post('/paypal/capture', requireCustomer, asyncRoute(async (req: Request, if (!rows.length) return res.status(404).json({ error: 'checkout not found' }); await client.query('BEGIN'); - const sold = await completeCheckout(client, rows[0].id, 'paypal', orderID, capture); + const sold = await completeCheckout(client, requireRow(rows, 'the checkout lookup').id, 'paypal', orderID, capture); await client.query('COMMIT'); await notifyFavoritersOfSale(sold.itemIds, sold.buyerId); res.json({ status: 'completed' }); @@ -282,7 +283,8 @@ webhookRouter.post('/', asyncRoute(async (req: Request, res: Response) => { const checkoutId = event.resource?.custom_id; if (checkoutId) { const { rows } = await pool.query(`SELECT status FROM checkouts WHERE id = $1`, [checkoutId]); - if (rows.length && rows[0].status !== 'completed') { + const [checkout] = rows; + if (checkout && checkout.status !== 'completed') { const client = await pool.connect(); try { await client.query('BEGIN'); diff --git a/backend/src/routes/customers.ts b/backend/src/routes/customers.ts index 07c71c1..20e7129 100755 --- a/backend/src/routes/customers.ts +++ b/backend/src/routes/customers.ts @@ -1,7 +1,7 @@ import { Router, Request, Response } from 'express'; import bcrypt from 'bcryptjs'; import crypto from 'node:crypto'; -import { pool } from '../db'; +import { pool, requireRow } from '../db'; import { requireCustomer } from '../middleware/customerAuth'; import { sendMail } from '../mailer'; import { renderTemplate, greeting, formatDuration } from '../emailTemplates'; @@ -220,7 +220,7 @@ router.post('/register', asyncRoute(async (req: Request, res: Response) => { unsubscribeToken ] ); - const customer = rows[0]; + const customer = requireRow(rows, 'the registration INSERT'); await issueVerificationEmail(customer.id, customer.email, customer.first_name, customer.last_name); @@ -235,8 +235,9 @@ router.post('/verify-email', asyncRoute(async (req: Request, res: Response) => { `SELECT * FROM customer_tokens WHERE token = $1 AND kind = 'verify_email' AND expires_at > now()`, [token] ); - if (!rows.length) return res.status(400).json({ error: 'invalid or expired token' }); - await pool.query(`UPDATE customers SET email_verified = true WHERE id = $1`, [rows[0].customer_id]); + const [verifyToken] = rows; + if (!verifyToken) return res.status(400).json({ error: 'invalid or expired token' }); + await pool.query(`UPDATE customers SET email_verified = true WHERE id = $1`, [verifyToken.customer_id]); await pool.query(`DELETE FROM customer_tokens WHERE token = $1`, [token]); res.json({ status: 'verified' }); })); @@ -250,7 +251,8 @@ router.post( verificationResendLimiter, asyncRoute(async (req: Request, res: Response) => { const { rows } = await pool.query(`SELECT * FROM customers WHERE id = $1`, [req.customerId]); - const customer = rows[0]; + // requireCustomer has already matched this id against a live session. + const customer = requireRow(rows, 'the signed-in customer'); // Refused rather than quietly sending. A pointless email is worse than an // answer, and the account page has no reason to offer the button here. @@ -323,8 +325,9 @@ router.post('/reset-password', asyncRoute(async (req: Request, res: Response) => `SELECT * FROM customer_tokens WHERE token = $1 AND kind = 'password_reset' AND expires_at > now()`, [token] ); - if (!rows.length) return res.status(400).json({ error: 'invalid or expired token' }); - const customerId = rows[0].customer_id; + const [resetToken] = rows; + if (!resetToken) return res.status(400).json({ error: 'invalid or expired token' }); + const customerId = resetToken.customer_id; // A token issued before the account was disabled would otherwise still mint a // fresh session. @@ -361,7 +364,7 @@ router.post('/reset-password', asyncRoute(async (req: Request, res: Response) => const { rows: fresh } = await pool.query(`SELECT * FROM customers WHERE id = $1`, [customerId]); const sessionToken = await createSession(customerId); setSessionCookie(res, sessionToken); - res.json(publicCustomer(fresh[0])); + res.json(publicCustomer(requireRow(fresh, 'the customer whose password was just reset'))); })); router.post('/login', asyncRoute(async (req: Request, res: Response) => { @@ -430,13 +433,13 @@ router.put('/me/favorite-alerts', requireCustomer, asyncRoute(async (req: Reques WHERE id = $4 RETURNING *`, [enabled, enabled ? new Date() : null, enabled ? FAVORITE_ALERTS_CONSENT_TEXT : null, req.customerId] ); - res.json(publicCustomer(rows[0])); + res.json(publicCustomer(requireRow(rows, 'the favorite-alerts UPDATE'))); })); router.get('/me', requireCustomer, asyncRoute(async (req: Request, res: Response) => { - const { rows } = await pool.query(`SELECT * FROM customers WHERE id = $1`, [req.customerId]); - if (!rows.length) return res.status(404).json({ error: 'not found' }); - res.json(publicCustomer(rows[0])); + const [customer] = await pool.query(`SELECT * FROM customers WHERE id = $1`, [req.customerId]).then(r => r.rows); + if (!customer) return res.status(404).json({ error: 'not found' }); + res.json(publicCustomer(customer)); })); // Kept in step with registration for consistency. Note nothing in the frontend @@ -459,7 +462,7 @@ router.put('/me', requireCustomer, asyncRoute(async (req: Request, res: Response `UPDATE customers SET first_name = $1, last_name = $2 WHERE id = $3 RETURNING *`, [first, last, req.customerId] ); - res.json(publicCustomer(rows[0])); + res.json(publicCustomer(requireRow(rows, 'the name UPDATE'))); })); router.post('/change-password', requireCustomer, asyncRoute(async (req: Request, res: Response) => { @@ -468,7 +471,7 @@ router.post('/change-password', requireCustomer, asyncRoute(async (req: Request, return res.status(400).json({ error: 'new password must be at least 8 characters' }); } const { rows } = await pool.query(`SELECT * FROM customers WHERE id = $1`, [req.customerId]); - const customer = rows[0]; + const customer = requireRow(rows, 'the signed-in customer'); if (!(await bcrypt.compare(currentPassword || '', customer.password_hash))) { return res.status(401).json({ error: 'current password is incorrect' }); } @@ -500,7 +503,7 @@ router.put('/me/email', requireCustomer, asyncRoute(async (req: Request, res: Re } const { rows } = await pool.query(`SELECT * FROM customers WHERE id = $1`, [req.customerId]); - const customer = rows[0]; + const customer = requireRow(rows, 'the signed-in customer'); if (!(await bcrypt.compare(String(currentPassword ?? ''), customer.password_hash))) { return res.status(401).json({ error: 'current password is incorrect' }); @@ -542,7 +545,7 @@ router.put('/me/email', requireCustomer, asyncRoute(async (req: Request, res: Re .catch(err => console.error('email change notice send failed', err)); const { rows: updated } = await pool.query(`SELECT * FROM customers WHERE id = $1`, [req.customerId]); - res.json(publicCustomer(updated[0])); + res.json(publicCustomer(requireRow(updated, 'the customer after the email change'))); })); router.post('/me/consent', requireCustomer, asyncRoute(async (req: Request, res: Response) => { @@ -569,7 +572,7 @@ router.get('/me/export', requireCustomer, asyncRoute(async (req: Request, res: R const { rows: orderRows } = await pool.query(`SELECT * FROM orders WHERE customer_id = $1`, [req.customerId]); res.setHeader('Content-Disposition', 'attachment; filename="my-data.json"'); res.json({ - customer: publicCustomer(customerRows[0]), + customer: publicCustomer(requireRow(customerRows, 'the signed-in customer')), orders: orderRows, exported_at: new Date().toISOString() }); diff --git a/backend/src/routes/public.ts b/backend/src/routes/public.ts index f2af8e0..84ca1ce 100755 --- a/backend/src/routes/public.ts +++ b/backend/src/routes/public.ts @@ -1,5 +1,5 @@ import { Router, Request, Response } from 'express'; -import { pool } from '../db'; +import { pool, requireRow } from '../db'; import { asyncRoute } from '../asyncRoute'; interface IdRow { @@ -18,7 +18,7 @@ router.get('/unsubscribe', asyncRoute(async (req: Request, res: Response) => { await pool.query( `UPDATE customers SET marketing_consent = false, marketing_consent_at = now(), marketing_consent_text = 'Unsubscribed via email link' WHERE id = $1`, - [rows[0].id] + [requireRow(rows, 'the unsubscribe-token lookup').id] ); res.send('

You\'ve been unsubscribed.

You will no longer receive marketing emails from Redefined Designs.

'); })); diff --git a/backend/src/utils.ts b/backend/src/utils.ts index 6a923b7..382a78d 100755 --- a/backend/src/utils.ts +++ b/backend/src/utils.ts @@ -43,7 +43,7 @@ export function isValidEmail(email: string): boolean { // antd's preset Tag colours. Kept as the single source of truth for tag // colours so the admin palette picker and the auto-assignment below can never // drift apart — the frontend renders whatever string lands in tags.color. -export const TAG_COLORS = [ +export const TAG_COLORS: [string, ...string[]] = [ 'magenta', 'red', 'volcano', 'orange', 'gold', 'lime', 'green', 'cyan', 'blue', 'geekblue', 'purple' ]; @@ -61,7 +61,14 @@ export function tagColorFor(name: string): string { for (let i = 0; i < normalized.length; i++) { hash = ((hash << 5) + hash + normalized.charCodeAt(i)) | 0; } - return TAG_COLORS[Math.abs(hash) % TAG_COLORS.length]; + // The modulo keeps this in range, but an index signature cannot say so. The + // fallback is the first colour rather than a throw: a tag with an unexpected + // colour is not worth failing a request over. + // TAG_COLORS is typed as a non-empty tuple, so index 0 is known to exist — + // the annotation, rather than `as const`, because the elements must stay + // `string` for the callers that assign them. The modulo keeps the computed + // index in range; the fallback only exists because indexing cannot say so. + return TAG_COLORS[Math.abs(hash) % TAG_COLORS.length] ?? TAG_COLORS[0]; } export const MARKETING_CONSENT_TEXT = diff --git a/backend/tests/integration/clientErrors.integration.test.ts b/backend/tests/integration/clientErrors.integration.test.ts index 204520c..0940e1d 100644 --- a/backend/tests/integration/clientErrors.integration.test.ts +++ b/backend/tests/integration/clientErrors.integration.test.ts @@ -32,7 +32,7 @@ describe('POST /api/client-errors', () => { expect(res.status).toBe(204); expect(errorSpy).toHaveBeenCalledTimes(1); - const logged = errorSpy.mock.calls[0][0] as string; + const logged = errorSpy.mock.calls[0]?.[0] as string; expect(logged).toContain('[client-error]'); expect(logged).toContain('context=catalogue'); expect(logged).toContain('Cannot read properties of undefined'); @@ -64,7 +64,7 @@ describe('POST /api/client-errors', () => { }); expect(res.status).toBe(204); - const logged = errorSpy.mock.calls[0][0] as string; + const logged = errorSpy.mock.calls[0]?.[0] as string; expect(logged).toContain('[truncated]'); expect(logged.length).toBeLessThan(2000); }); @@ -76,7 +76,7 @@ describe('POST /api/client-errors', () => { const res = await request(app).post('/api/client-errors').send({ context: 'page', message }); expect(res.status).toBe(204); - const logged = errorSpy.mock.calls[0][0] as string; + const logged = errorSpy.mock.calls[0]?.[0] as string; expect(logged).toContain(`message: ${message}\n`); expect(logged).not.toContain('[truncated]'); }); @@ -87,7 +87,7 @@ describe('POST /api/client-errors', () => { const res = await request(app).post('/api/client-errors').send({ context: 'page', message }); expect(res.status).toBe(204); - const logged = errorSpy.mock.calls[0][0] as string; + const logged = errorSpy.mock.calls[0]?.[0] as string; expect(logged).toContain(`message: ${'a'.repeat(500)}… [truncated]`); }); @@ -99,7 +99,7 @@ describe('POST /api/client-errors', () => { }); expect(res.status).toBe(204); - const logged = errorSpy.mock.calls[0][0] as string; + const logged = errorSpy.mock.calls[0]?.[0] as string; const stackLine = logged.split('\n').find((line) => line.trim().startsWith('stack:')); expect(stackLine).toContain('[truncated]'); }); @@ -112,7 +112,7 @@ describe('POST /api/client-errors', () => { }); expect(res.status).toBe(204); - const logged = errorSpy.mock.calls[0][0] as string; + const logged = errorSpy.mock.calls[0]?.[0] as string; const componentStackLine = logged .split('\n') .find((line) => line.trim().startsWith('componentStack:')); @@ -127,7 +127,7 @@ describe('POST /api/client-errors', () => { }); expect(res.status).toBe(204); - const logged = errorSpy.mock.calls[0][0] as string; + const logged = errorSpy.mock.calls[0]?.[0] as string; const pathLine = logged.split('\n')[0]; expect(pathLine).toContain('[truncated]'); }); @@ -142,7 +142,7 @@ describe('POST /api/client-errors', () => { }); expect(res.status).toBe(204); - const logged = errorSpy.mock.calls[0][0] as string; + const logged = errorSpy.mock.calls[0]?.[0] as string; // The template itself joins four fixed lines with three newlines; that // count must not grow no matter what the caller sends. diff --git a/backend/tests/integration/resendVerification.integration.test.ts b/backend/tests/integration/resendVerification.integration.test.ts index 9e23535..fbbffca 100644 --- a/backend/tests/integration/resendVerification.integration.test.ts +++ b/backend/tests/integration/resendVerification.integration.test.ts @@ -67,7 +67,7 @@ describe('resending your own verification email', () => { expect(res.status).toBe(204); expect(sentMail).toHaveBeenCalledTimes(1); - expect(String(sentMail.mock.calls[0][0])).toBe(email); + expect(String(sentMail.mock.calls[0]?.[0])).toBe(email); }); // The point of the whole thing. An un-superseded link means a message still diff --git a/backend/tests/unit/adminGate.test.ts b/backend/tests/unit/adminGate.test.ts index 95fec7f..122c898 100644 --- a/backend/tests/unit/adminGate.test.ts +++ b/backend/tests/unit/adminGate.test.ts @@ -131,7 +131,7 @@ describe('requireAdminGate', () => { requireAdminGate(h.req, h.res, h.next); expect(warn).toHaveBeenCalledTimes(1); - const logged = warn.mock.calls[0][0] as string; + const logged = warn.mock.calls[0]?.[0] as string; expect(logged).toContain('[admin-gate]'); expect(logged).toContain('/api/admin/items'); expect(logged).not.toContain('some-guessed-value'); diff --git a/backend/tests/unit/composeEnvironment.test.ts b/backend/tests/unit/composeEnvironment.test.ts index 5af872a..a9fb363 100644 --- a/backend/tests/unit/composeEnvironment.test.ts +++ b/backend/tests/unit/composeEnvironment.test.ts @@ -75,8 +75,11 @@ function environmentEntries(source: string): Map { // this guard was found to be broken. for (const line of source.split(/\r?\n/)) { const match = /^\s+- ([A-Z_0-9]+)=(.*)$/.exec(line); - if (match) { - entries.set(match[1], match[2].trim()); + // Both groups are non-optional in the pattern, so a match always has them — + // but RegExpExecArray cannot say so, and #101 made the compiler insist. + const [, name, value] = match ?? []; + if (name !== undefined && value !== undefined) { + entries.set(name, value.trim()); } } return entries; diff --git a/backend/tsconfig.json b/backend/tsconfig.json index f77b062..fd3bc76 100755 --- a/backend/tsconfig.json +++ b/backend/tsconfig.json @@ -6,6 +6,11 @@ "outDir": "dist", "rootDir": "src", "strict": true, + // #101. Indexing an array gives `T | undefined`, which is what it always + // was — this makes the compiler say so. Only worth having once query rows + // carry real types (#159): before that `rows[0]` was `any`, and `any` + // indexes to `any`, so there was nothing for this to check. + "noUncheckedIndexedAccess": true, "esModuleInterop": true, "skipLibCheck": true, "resolveJsonModule": true diff --git a/frontend/src/admin/Tags.tsx b/frontend/src/admin/Tags.tsx index 55f6d4d..a472d6f 100644 --- a/frontend/src/admin/Tags.tsx +++ b/frontend/src/admin/Tags.tsx @@ -15,7 +15,7 @@ const { Title } = Typography; // Mirrors TAG_COLORS in the backend's utils.ts — the server rejects anything // outside this set, so the two lists have to stay aligned. -const TAG_COLORS = [ +const TAG_COLORS: [string, ...string[]] = [ 'magenta', 'red', 'volcano', 'orange', 'gold', 'lime', 'green', 'cyan', 'blue', 'geekblue', 'purple' ]; diff --git a/frontend/src/components/FilterDrawer.tsx b/frontend/src/components/FilterDrawer.tsx index 3555419..673e02d 100644 --- a/frontend/src/components/FilterDrawer.tsx +++ b/frontend/src/components/FilterDrawer.tsx @@ -166,7 +166,15 @@ export default function FilterDrawer({ value={[filters.minPriceCents ?? bounds.min_cents, filters.maxPriceCents ?? sliderMax]} tooltip={{ formatter: (value) => `$${((value ?? 0) / 100).toFixed(0)}` }} onChange={([min, max]) => - onChange({ ...filters, minPriceCents: min, maxPriceCents: max }) + // antd types the slider's value as number[], so destructuring gives + // `number | undefined`. A range slider always emits both ends; the + // fallbacks are the bounds it was given rather than nulls, which + // would read as "no filter" and widen the results. + onChange({ + ...filters, + minPriceCents: min ?? bounds.min_cents, + maxPriceCents: max ?? sliderMax + }) } />
diff --git a/frontend/tests/e2e/admin-theme.spec.ts b/frontend/tests/e2e/admin-theme.spec.ts index 8e1a2bd..3d1ccc3 100644 --- a/frontend/tests/e2e/admin-theme.spec.ts +++ b/frontend/tests/e2e/admin-theme.spec.ts @@ -3,7 +3,9 @@ import { test, expect, AdminPage, createCategory, createTag, uniqueSuffix } from // Relative luminance per WCAG, used to tell "light" from "dark" without // asserting exact hex values, which would break on any palette tweak. function luminance(rgb: string): number { - const [r, g, b] = (rgb.match(/\d+(\.\d+)?/g) ?? ['0', '0', '0']).slice(0, 3).map(Number); + // Defaulted per channel rather than on the match: a colour string with + // fewer than three numbers would otherwise leave a channel undefined. + const [r = 0, g = 0, b = 0] = (rgb.match(/\d+(\.\d+)?/g) ?? []).slice(0, 3).map(Number); const channel = (c: number) => { const s = c / 255; return s <= 0.03928 ? s / 12.92 : Math.pow((s + 0.055) / 1.055, 2.4); diff --git a/frontend/tsconfig.json b/frontend/tsconfig.json index 8b0351e..3720f93 100755 --- a/frontend/tsconfig.json +++ b/frontend/tsconfig.json @@ -7,6 +7,9 @@ "moduleResolution": "bundler", "jsx": "react-jsx", "strict": true, + // #101. Indexing gives `T | undefined`, which it always did — this makes + // the compiler say so. + "noUncheckedIndexedAccess": true, "skipLibCheck": true, "esModuleInterop": true, "noEmit": true diff --git a/frontend/tsconfig.sonar.json b/frontend/tsconfig.sonar.json index f808215..1568192 100644 --- a/frontend/tsconfig.sonar.json +++ b/frontend/tsconfig.sonar.json @@ -8,6 +8,9 @@ "moduleResolution": "node", "jsx": "react-jsx", "strict": true, + // #101. Indexing gives `T | undefined`, which it always did — this makes + // the compiler say so. + "noUncheckedIndexedAccess": true, "skipLibCheck": true, "esModuleInterop": true, "noEmit": true