From 7e5c16446fb7aece82dc67af465434940ef5b486 Mon Sep 17 00:00:00 2001 From: Matthew Raymer Date: Fri, 15 Aug 2025 09:07:49 +0000 Subject: [PATCH] docs(architecture): enhance component creation MDC rules with concision framework - Added Concision-First Decision Framework to guide component creation - Included Rule of Three guardrail for when to extract components - Added practical PR language guidance for code reviews - Enhanced agent role to prefer concision where appropriate - Restored priority levels, cross-references, and version history - Updated examples to reflect successful NotificationSection refactor - Enhanced .cursor/rules/component-creation-ideals.mdc to v2.0.0 - Added comprehensive migration patterns and testing guidelines - Established architectural standards for future component development --- .cursor/rules/component-creation-ideals.mdc | 321 ++++++++++++++++++++ 1 file changed, 321 insertions(+) create mode 100644 .cursor/rules/component-creation-ideals.mdc diff --git a/.cursor/rules/component-creation-ideals.mdc b/.cursor/rules/component-creation-ideals.mdc new file mode 100644 index 00000000..661da418 --- /dev/null +++ b/.cursor/rules/component-creation-ideals.mdc @@ -0,0 +1,321 @@ +--- +alwaysApply: true +version: "2.0.0" +lastUpdated: "2025-08-15" +priority: "critical" +--- +# Component Creation Ideals β€” TimeSafari Architecture (Directive MDC, v2) + +> **Agent role**: Apply these rules when creating, refactoring, or reviewing Vue components (Vue 3 with **vue-facing-decorator**). Prioritize **self-contained** components that hydrate from and persist to **PlatformServiceMixin** settings. Minimize parent–child coupling and prop drilling. Prefer **concision** where a separate component would add more API surface than value. + +## πŸ“š Cross-References + +- **Migration Example**: See `src/components/NotificationSection.vue` for successful refactor implementation +- **Parent Component**: See `src/views/AccountViewView.vue` for before/after comparison +- **Settings Infrastructure**: See `src/utils/PlatformServiceMixin.ts` for available methods +- **Related Rules**: See `.cursor/rules/` for other architectural guidelines + +## Golden Rules (Enforce) + +### Priority Levels +- πŸ”΄ **CRITICAL**: Must be followed - breaking these creates architectural problems +- 🟑 **HIGH**: Strongly recommended - important for maintainability +- 🟒 **MEDIUM**: Good practice - improves code quality + +1. **Self-Contained Components** πŸ”΄ **CRITICAL** + - Do **not** require parent props for internal state or behavior. + - Hydrate on `mounted()` via `this.$accountSettings()`; persist with `this.$saveSettings()`. + - Encapsulate business logic inside the component; avoid delegating core logic to the parent. + - Prefer **computed** getters for derived values; avoid stored duplicates of derived state. + +2. **Settings-First Architecture** πŸ”΄ **CRITICAL** + - Use `PlatformServiceMixin` for reading/writing settings. Do **not** introduce new state managers (Pinia, custom stores) unless the state is *truly global*. + - Prefer **fetch-on-mount** over passing values via props. + +3. **Single Responsibility** 🟑 **HIGH** + - Each component owns **one clear purpose** (UI + related logic + settings persistence). Avoid splitting UI/logic across multiple components unless reusability clearly benefits. + +4. **Internal State Lifecycle** 🟑 **HIGH** + - Pattern: **defaults β†’ hydrate on mount β†’ computed for derived β†’ persist on change**. + - Handle hydration errors gracefully (keep safe defaults; surface actionable UI states as needed). + +5. **Minimal Props** πŸ”΄ **CRITICAL** + - Props are for **pure configuration** (labels, limits, feature flags). Do not pass data that can be loaded internally. + - **Never** pass props that mirror settings values (e.g., `isRegistered`, `notifying*`). Load those from settings. + +6. **Communication & Events** 🟑 **HIGH** + - Children may emit events for *user interactions* (e.g., `submitted`, `closed`) but **not** to offload core logic to the parent. + - Do not emit events solely to persist settings; the child handles persistence. + +7. **Testing** 🟒 **MEDIUM** + - Unit tests mount components in isolation; mock `this.$accountSettings`/`this.$saveSettings`. + - Verify hydration on mount, persistence on mutation, and graceful failure on settings errors. + +--- + +## Concision-First Decision Framework + +> **Goal:** Avoid unnecessary components when a concise script, composable, or helper suffices. + +**Prefer NOT making a component when:** +- **One-off UI**: used in exactly one view, unlikely to repeat. +- **Small scope**: ~≀100–150 LOC, ≀3 reactive fields, ≀2 handlers. +- **Purely presentational** with trivial logic. +- **Local invariants**: behavior depends entirely on the view’s context. +- **Abstraction cost > benefit**: would create an anemic component with props mirroring parent state. +- **Better fit as code reuse**: logic works as a **composable/service/helper** without introducing new UI. + +**Concise alternatives:** +- **Composable**: `useFeature()` encapsulates settings I/O and state. +- **Service/Module**: plain TS helpers for formatting/validation. +- **Directive**: tiny DOM behaviors that don’t need a lifecycle boundary. + +**When to make a component (even without reuse yet):** +- **Isolation boundary**: async side effects, permission prompts, or recoverable error states. +- **Stateful widget**: internal settings persistence, media controls, complex a11y. +- **Slots/composition**: needs flexible children or layout. +- **Different change rate**: sub-tree churns independently of the parent. +- **Testability/ownership**: clear, ownable surface that’s easier to unit-test in isolation. + +**Rule of Three (guardrail):** +- 1st time: inline or composable. +- 2nd time: consider shared abstraction. +- 3rd time: extract a component. + +**PR language (use when choosing concision):** +- β€œOne-off, ~80 LOC, no expected reuse. A component would add an API surface with no consumer. Keeping it local reduces cognitive load. If we see a second usage, promote to a composable; third usage, a component.” +- β€œLogic lives in `useX()` to keep the view concise without prop plumbing; settings stay via `PlatformServiceMixin`.” + +--- + +## Anti-Patterns (Reject) + +- Props that duplicate internal/derivable state: `:is-registered`, `:notifying-*`, etc. +- Child components that are **UI-only** while parents hold the business logic for that feature. +- Introducing Pinia/custom stores for per-component state. +- Emitting `update:*` events to push settings responsibilities to parents. +- Scattering a feature across multiple micro-components without a clear reuse reason. +- Using props for computed/derived values (e.g., `showAdvancedFeatures` as a prop). + +--- + +## Quick Examples + +### βœ… DO (Self-Contained, Settings-First) + +```vue + + +``` + +```ts +// NotificationSection.vue (vue-facing-decorator + PlatformServiceMixin) +import { Component, Vue } from "vue-facing-decorator"; +import { PlatformServiceMixin } from "@/utils/PlatformServiceMixin"; + +@Component({ + mixins: [PlatformServiceMixin], +}) +export default class NotificationSection extends Vue { + private notifyingNewActivity: boolean = false; + + async mounted(): Promise { + await this.hydrateFromSettings(); + } + + private async hydrateFromSettings(): Promise { + try { + const s = await this.$accountSettings(); + this.notifyingNewActivity = !!s.notifyingNewActivityTime; + } catch (err) { + // Keep defaults; optionally surface a non-blocking UI notice + } + } + + private async updateNotifying(state: boolean): Promise { + await this.$saveSettings({ notifyingNewActivityTime: state ? Date.now() : null }); + this.notifyingNewActivity = state; + } +} +``` + +### ❌ DON'T (Prop-Driven Coupling) + +```vue + + +``` + +```ts +// Child receives props (anti-pattern) +export default class NotificationSection extends Vue { + isRegistered: boolean = false; + notifyingNewActivity: boolean = false; +} +``` + +--- + +## Component Structure Template (Drop-In) + +```ts +/** + * ComponentName.vue β€” Purpose + * Owns: UI + logic + settings persistence (PlatformServiceMixin). + */ +import { Component, Vue } from "vue-facing-decorator"; +import { PlatformServiceMixin } from "@/utils/PlatformServiceMixin"; +import type { Router } from "vue-router"; + +@Component({ + components: { + // child components here + }, + mixins: [PlatformServiceMixin], // Settings access +}) +export default class ComponentName extends Vue { + // Internal state + private myState: boolean = false; + private myData: string = ""; + + // Derived + private get isEnabled(): boolean { + return this.myState && this.hasPermissions; + } + + async mounted(): Promise { + await this.hydrateFromSettings(); + } + + private async hydrateFromSettings(): Promise { + try { + const s = await this.$accountSettings(); + this.myState = !!s.mySetting; + this.myData = s.myData ?? ""; + } catch (err) { + // keep defaults + } + } + + private async updateState(v: boolean): Promise { + await this.$saveSettings({ mySetting: v }); + this.myState = v; + } + + async handleUserAction(): Promise { + await this.updateState(true); + } +} +``` + +--- + +## Migration Playbook (Props β†’ Self-Contained) + +> **Agent, when you see a component receiving `@Prop()` values that mirror settings or internal state, apply this playbook.** + +1. **Detect Anti-Props** + - Flag props matching: `is*`, `has*`, `notifying*`, or mirroring settings keys. + - Search for parent usage of these props and events. + +2. **Inline State** + - Remove anti-props and `@Prop()` declarations. + - Add private fields for state inside the child. + - Add `mounted()` hydration and persistence helpers via `PlatformServiceMixin`. + +3. **Parent Simplification** + - Replace `` with ``. + - Delete now-unused local state, watchers, and computed values that existed only to feed the child. + +4. **Events** + - Keep only user-interaction events (e.g., `submitted`). Remove persistence/logic events that the child now owns. + +5. **Tests** + - Update unit tests to mount child in isolation; mock settings I/O. + - Verify: hydrate on mount, persist on change, safe fallback on error. + +--- + +## When Exceptions Are Acceptable + +- **Truly Global State** across distant components β†’ use Pinia/service *sparingly*. +- **Reusable Form Inputs** β†’ accept `value` prop and emit `input`/`update:modelValue`; keep validation/business logic internal. +- **Configuration-Only Props** for labels, visual variants, or limits β€” not for state that can be fetched. + +--- + +## Definition of Done (Checklist) + +- [ ] No props required for internal state or settings-backed values. +- [ ] Uses `PlatformServiceMixin` for all settings I/O. +- [ ] Hydrates on `mounted()`, persists on mutations. +- [ ] Single-responsibility: UI + logic + persistence together. +- [ ] Computed getters for derived state. +- [ ] Unit tests mock settings and cover hydrate/persist/failure paths. +- [ ] Parent components contain no leftover `notifying-*` or similar prop wiring. + +--- + +## Testing Snippets + +```ts +// Hydration on mount +test("hydrates from settings on mount", async () => { + const wrap = mount(MyComponent); + await wrap.vm.$nextTick(); + expect((wrap.vm as any).myState).toBe(true); +}); +``` + +```ts +// Mock settings methods +jest.spyOn(wrapper.vm as any, "$accountSettings").mockResolvedValue({ mySetting: true }); +jest.spyOn(wrapper.vm as any, "$saveSettings").mockResolvedValue(void 0); +``` + +```ts +// Graceful failure +test("handles settings load failure", async () => { + jest.spyOn(wrapper.vm as any, "$accountSettings").mockRejectedValue(new Error("DB Error")); + const wrap = mount(MyComponent); + await wrap.vm.$nextTick(); + expect((wrap.vm as any).myState).toBe(false); // default +}); +``` + +--- + +## Rationale (Short) + +- **Concision first where appropriate** β†’ avoid unnecessary components and API surfaces. +- **Reduced coupling** β†’ portability and reuse when boundaries are justified. +- **Maintainability** β†’ changes localized to the owning component. +- **Consistency** β†’ one canonical path for settings-backed features. + +**Rule of thumb:** _Can this feature operate independently, and does a component materially improve isolation or testability?_ If not, keep it concise (inline or composable). + +--- + +## πŸ“ Version History + +### v2.0.0 (2025-08-15) +- **Major Enhancement**: Added Concision-First Decision Framework +- **New**: Rule of Three guardrail for component extraction +- **New**: PR language guidance for code reviews +- **Enhanced**: Agent role includes concision preference +- **Refined**: Anti-patterns and examples updated + +### v1.0.0 (2025-08-15) +- Initial creation based on successful NotificationSection refactor +- Established core architectural principles for self-contained components +- Added comprehensive migration playbook and testing guidelines +- Included practical examples and anti-pattern detection + +### Future Enhancements +- Additional migration patterns for complex components +- Integration with automated refactoring tools +- Performance benchmarking guidelines +- Advanced testing strategies for complex state management