Browse Source
- 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 developmentpull/150/head
1 changed files with 321 additions and 0 deletions
@ -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 |
|||
<!-- Parent renders with no props --> |
|||
<NotificationSection /> |
|||
``` |
|||
|
|||
```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<void> { |
|||
await this.hydrateFromSettings(); |
|||
} |
|||
|
|||
private async hydrateFromSettings(): Promise<void> { |
|||
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<void> { |
|||
await this.$saveSettings({ notifyingNewActivityTime: state ? Date.now() : null }); |
|||
this.notifyingNewActivity = state; |
|||
} |
|||
} |
|||
``` |
|||
|
|||
### ❌ DON'T (Prop-Driven Coupling) |
|||
|
|||
```vue |
|||
<!-- Parent passes internal state down --> |
|||
<NotificationSection |
|||
:is-registered="isRegistered" |
|||
:notifying-new-activity="notifyingNewActivity" |
|||
/> |
|||
``` |
|||
|
|||
```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<void> { |
|||
await this.hydrateFromSettings(); |
|||
} |
|||
|
|||
private async hydrateFromSettings(): Promise<void> { |
|||
try { |
|||
const s = await this.$accountSettings(); |
|||
this.myState = !!s.mySetting; |
|||
this.myData = s.myData ?? ""; |
|||
} catch (err) { |
|||
// keep defaults |
|||
} |
|||
} |
|||
|
|||
private async updateState(v: boolean): Promise<void> { |
|||
await this.$saveSettings({ mySetting: v }); |
|||
this.myState = v; |
|||
} |
|||
|
|||
async handleUserAction(): Promise<void> { |
|||
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 `<Child :foo="..." :bar="..." @update="..."/>` with `<Child />`. |
|||
- 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 |
Loading…
Reference in new issue