From 6b70cca58c1fcacb9127e973968124beed34673d Mon Sep 17 00:00:00 2001 From: Randall Stillwell Date: Tue, 26 May 2026 22:50:13 -0500 Subject: [PATCH] chore(components): remove dead user prop from MobileNavigation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit components/MobileNavigation.js has accepted a `user` prop ever since the mobile bottom-bar was extracted from Layout, but it has never read any field of `user`. The bottom-bar items (Cards, Decks, Dashboard, Community, More) are statically configured — none of them branch on auth state, role, user id, or any other per-user attribute. The prop is dead. This was originally surfaced as R8 in the fix-layout-default-user convoy (commit ca302a8) and deliberately deferred there to keep that convoy focused on the Layout default-user fix. The follow-up was queued as cleanup-mobile-nav-dead-props in .convoys/ship-readiness.md § Queued convoys. Pre-edit audit confirms the queue entry's premise: `rg '\\buser\\b' components/MobileNavigation.js` returns 1 hit (the destructure on line 5) before the change and 0 hits after. The only active call site is components/Layout.js line 598; the components/Layout.js.backup snapshot also calls it but is a no-go-zone (per .cursor/rules/no-go-zones.mdc § "Append-only / historical") and stays untouched — when that backup is eventually deleted in a separate convoy, its stale call disappears with it. Verification: npm run lint exit 1 with 128 problems (baseline preserved, no regression introduced); npm run test:run 21/21 pass (test/components/Layout.test.js still asserts the logged-out branch contract from PR #15 — the dead-prop removal is invisible to that suite since it does not inspect MobileNavigation's prop shape). Convoy file .convoys/cleanup-mobile-nav-dead-props.md captures the audit, fix, risks (R1: a future per-user bottom-bar feature would need to re-add the prop — accepted; carrying dead state to hedge hypothetical features is worse than paying the one-line re-add cost when the feature actually lands), and acceptance criteria. Co-authored-by: Cursor --- .convoys/cleanup-mobile-nav-dead-props.md | 155 ++++++++++++++++++++++ components/Layout.js | 1 - components/MobileNavigation.js | 2 +- 3 files changed, 156 insertions(+), 2 deletions(-) create mode 100644 .convoys/cleanup-mobile-nav-dead-props.md diff --git a/.convoys/cleanup-mobile-nav-dead-props.md b/.convoys/cleanup-mobile-nav-dead-props.md new file mode 100644 index 0000000..1e75eb1 --- /dev/null +++ b/.convoys/cleanup-mobile-nav-dead-props.md @@ -0,0 +1,155 @@ +# cleanup-mobile-nav-dead-props (P3 polish — single-prop hygiene) + +**Status:** SHIPPED 2026-05-26 (PR TBD) +**Classification:** hygiene +**Priority:** P3 polish (not a bug, not a security issue; dead-prop +removal is purely a clarity-of-surface cleanup) +**Convoy owner:** parent (no architect — single-line prop removal in +one component + one caller; surfaced and pre-decided in a sibling +convoy) +**Opened:** 2026-05-26 + +## Background + +`components/MobileNavigation.js` accepts `{ user, onMenuOpen }` but +never reads `user.*` — the bottom-bar items (Cards, Decks, Dashboard, +Community, More) are static and don't depend on auth state or role. + +This was originally surfaced as **R8** in the `fix-layout-default-user` +convoy (see `.convoys/fix-layout-default-user.md` § R8 and § "Anything +flagged but not acted on") and deferred there with explicit +instructions: *"If the implementer is tempted to delete the prop, they +MUST stop — that's god-component-split / single-auth-provider +territory."* The deferral was correct for that convoy's scope; it is +no longer needed because the prop is genuinely dead at the current +static-bar reality, and removing it does not require a wider auth +refactor. + +The follow-up was queued as `cleanup-mobile-nav-dead-props` in +`.convoys/ship-readiness.md` § Queued convoys, with a note that it +may fold into `god-component-split` (P2 #13) if that lands first. +God-component-split has not landed; this small hygiene convoy ships +first. + +## Audit results + +Pre-edit audit (the spec's "don't blindly trust the queue entry" +clause): + +1. **Reading `components/MobileNavigation.js`** — the file is 171 + lines. Line 5 destructures `{ user, onMenuOpen }`. Lines 6–170 use + `onMenuOpen` exactly once (line 39, as the `onClick` for the "More" + button). `user` does not appear elsewhere — no `user.email`, + `user.role`, `user.id`, no conditional render gated on `user`, no + pass-through to a child component. The bottom-bar `navigationItems` + array is hardcoded and does not branch on auth state. +2. **`rg '\buser\b' components/MobileNavigation.js`** before edit: 1 + hit (the destructure on line 5). After edit: 0 hits. +3. **`rg "MobileNavigation" components/ pages/ --type js`**: two + import + JSX-callsite pairs in the codebase: + - `components/Layout.js` (active) — line 5 import, line 598-601 JSX + call passing `user={user}` and `onMenuOpen={...}`. + - `components/Layout.js.backup` (no-go-zone per + `.cursor/rules/no-go-zones.mdc` § "Append-only / historical" — + "legacy snapshot; delete with a real PR, never edit") — line 5 + import, line 264 JSX call. Left untouched per the no-go-zone + rule; if/when the `.backup` file is eventually deleted, this dead + call disappears with it. + +Audit verdict: `user` is genuinely dead. Cleanup is safe. + +## The fix + +Two-file, three-line diff: + +1. **`components/MobileNavigation.js` line 5**: remove `user` from the + destructured props. + - Before: `export default function MobileNavigation({ user, onMenuOpen }) {` + - After: `export default function MobileNavigation({ onMenuOpen }) {` +2. **`components/Layout.js` lines 598-601**: remove the `user={user}` + JSX attribute from the only active call site. + - Before: + ``` + setIsMobileMenuOpen(true)} + /> + ``` + - After: + ``` + setIsMobileMenuOpen(true)} + /> + ``` + +No new code. No refactors. No tests added (the component has no +direct test coverage; `test/components/Layout.test.js` tests Layout's +logged-out branch and does not assert on `MobileNavigation`'s prop +shape). + +## Out of scope + +- The unused `import { useState } from 'react'` on + `components/MobileNavigation.js` line 3. The hook is imported but + not called. This is a pre-existing dead import unrelated to the + `user` prop; the convoy spec explicitly forbids "refactor anything + else in MobileNavigation.js (this is a single-prop removal)". A + future hygiene pass can sweep it (or it'll get caught by an + eventual lint-no-unused-imports rule). +- `components/Layout.js.backup` — no-go-zone, untouched. + +## Verification plan + +1. `rg '\buser\b' components/MobileNavigation.js` → 0 hits (post-edit + confirmation that the prop is truly gone, not just renamed). +2. `rg "MobileNavigation" components/ pages/ --type js` → confirm + each active call site passes only `onMenuOpen`. +3. `npm run lint` → 128 problems baseline preserved (no regression + introduced; no new dead-code/unused-var warnings created by the + change). +4. `npm run test:run` → 21/21 pass. Specifically, + `test/components/Layout.test.js` continues to pass — its + regression-lock assertions for the logged-out Layout branch + (Gotcha #8) do not depend on `MobileNavigation`'s prop shape, so + the dead-prop removal is invisible to that suite. +5. `npm run build` skipped — relying on Vercel preview CI. Trade-off: + single-prop removal in a leaf component is extremely low risk of + build-time regression, and the Playwright smoke + visual-diff + workflows on the PR will catch any Layout-rendering issue before + merge. + +## Risks + +- **R1 — A future feature that wants per-user bottom-bar items would + need to re-add the prop.** Hypothetical examples: showing an + unread-count badge on a "Notifications" tab gated on `user.id`, or + hiding the "Community" tab for unauthenticated visitors. **Accepted.** + Re-adding a prop is a one-line change when the feature actually + lands; carrying a dead prop "just in case" obscures the current + surface and adds nothing. The cleanup is correct for the + current static-bar reality; future features pay their own + add-the-prop cost. +- **R2 — `components/Layout.js.backup` still references the old prop + shape.** **Accepted.** The backup is a no-go-zone (per + `.cursor/rules/no-go-zones.mdc`) and is dead code by definition. + Touching it would violate the rule; leaving it as a stale snapshot + is the convention. When the backup is eventually deleted in a + separate convoy, this stale call disappears with it. + +## Acceptance criteria + +- 2 files modified, 3 lines net change (1 line edit + 1 attribute + removal from a multi-line JSX block). +- `npm run lint` exit 1 with 128 problems (baseline preserved). +- `npm run test:run` 21/21 pass. +- `rg '\buser\b' components/MobileNavigation.js` → 0 hits post-edit. + +## Owns + +Parent (single-prop removal in a leaf component; no architect or +implementer subagent required; audit confirms the queue entry's +premise). + +## As-shipped + +_To be filled in post-merge._ diff --git a/components/Layout.js b/components/Layout.js index d9472e1..84a14fa 100644 --- a/components/Layout.js +++ b/components/Layout.js @@ -596,7 +596,6 @@ export default function Layout({ children, user = null, showSearch = false }) {
{/* Mobile Navigation - Bottom bar for mobile */} setIsMobileMenuOpen(true)} /> diff --git a/components/MobileNavigation.js b/components/MobileNavigation.js index c492495..56690ae 100644 --- a/components/MobileNavigation.js +++ b/components/MobileNavigation.js @@ -2,7 +2,7 @@ import { useRouter } from 'next/router'; import Link from 'next/link'; import { useState } from 'react'; -export default function MobileNavigation({ user, onMenuOpen }) { +export default function MobileNavigation({ onMenuOpen }) { const router = useRouter(); // Navigation items for the bottom bar -- 2.45.2