deckhearth/.convoys/cleanup-mobile-nav-dead-props.md
Randall Stillwell 6b70cca58c chore(components): remove dead user prop from MobileNavigation
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 <cursoragent@cursor.com>
2026-05-26 22:50:13 -05:00

155 lines
6.5 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# 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 6170 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:
```
<MobileNavigation
user={user}
onMenuOpen={() => setIsMobileMenuOpen(true)}
/>
```
- After:
```
<MobileNavigation
onMenuOpen={() => 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._