refactor(frontend): seal readonly leaks in app-types consumers (#1596)#1598
Conversation
Finish the app-types readonly contract opened by #1595, same class as the graph-type follow-ups #1593/#1594 (#1597): - AllCampersView: drop the now-redundant `as Camper[]` cast — #1595 widened buildCamperRows to accept `readonly Camper[]`, and `MergedCamper[]` is directly assignable, so the cast only suppressed future compiler signal. - mergeMultiSessionCampers: mark `AdditionalSession` fields and `MergedCamper.additionalSessions` `readonly` to match the now-`readonly` `Camper` it extends. The builder maps fresh objects, so the element type going readonly is a clean change. Pure type-only hardening — no runtime behavior change. Adds the first tests for mergeMultiSessionCampers (behavioral merge coverage + a tsc-enforced readonly contract via @ts-expect-error). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR completes the readonly immutability contract for MergedCamper and its AdditionalSession fields. Type contracts are tightened to enforce readonly semantics, comprehensive tests validate both runtime merge behavior and compile-time type safety, and a now-redundant type cast is removed from the CSV export path. ChangesReadonly contract sealing
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Assessment against linked issues
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
Closes #1596.
Finishes the app-types
readonlycontract opened by #1595 — same latent readonly-leak class as the graph-type follow-ups #1593/#1594 (shipped in #1597). Two consumers were intentionally left out of #1595 to keep that a pure type-only change.Changes
AllCampersView.tsx— drop the now-redundantas Camper[]cast. refactor(frontend): readonly on app-types cache/API shapes #1595 widenedbuildCamperRowsto acceptreadonly Camper[](csvExportHelpers.ts:73), andfilteredCampersisMergedCamper[](MergedCamper extends Camper), so it's directly assignable. The standing cast only suppressed future compiler signal if the contract changed.mergeMultiSessionCampers.ts— markAdditionalSession's fields andMergedCamper.additionalSessionsreadonly, matching the now-readonlyCamperthatMergedCamperextends. The builder constructs eachAdditionalSessionvia.map()into fresh objects, so the element type goingreadonlyis a clean change.Pure type-only hardening — no runtime behavior change, no active mutation existed.
Tests
Adds the first tests for
mergeMultiSessionCampers(previously untested):additionalSessions; single-session camper untouched.@ts-expect-errorassertions enforced bytsc --noEmit. Verified red→green: before the change the directives are unused (TS2578); after, they type-check.🤖 Generated with Claude Code
Summary by CodeRabbit
Tests
Refactor