Skip to content

Commit 73ad58f

Browse files
authored
Feat/media settings (#100)
* feat: App settings for OMDB and Streaming * fix redirects, cleanups * cr fixes * rule updates * tests update * command updates, prettier fixes
1 parent 9302b33 commit 73ad58f

23 files changed

+3605
-1571
lines changed

.cursor/commands/review-changes.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,14 @@ For each issue, be specific: file, line, problem, suggested fix.
6464

6565
**4. Pull Request Description:** Generate as a copyable markdown code block with relevant emojis per header.
6666

67+
**5. Learnings & Rule Updates:**
68+
69+
At the end of the review, summarize any patterns or learnings that emerged from the review (or from user feedback during the review process) that could be codified into rules. Ask the user if they would like to:
70+
71+
- Update an existing rule file in `.cursor/rules/`
72+
- Create a new rule file for the pattern
73+
- Skip persisting the learning
74+
75+
This ensures that recurring feedback and discovered patterns are captured for future development.
76+
6777
**Style:** Be critical, specific, and concise. No fluff. If unsure, ask for clarification.

.cursor/rules/FRONTEND_PATTERNS.mdc

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,89 @@ const MyComponent = Shade<{ prop: string }>({
6868
- **Loading states**: Boolean observables for async operations
6969
- **Error states**: String observables for error messages
7070

71+
## DOM Access in Shades Components
72+
73+
### Avoid Global Document Queries
74+
75+
Never use `document.querySelector()` in Shades components. It bypasses shadow DOM boundaries and may not find elements correctly.
76+
77+
```typescript
78+
// ❌ Bad - using document.querySelector
79+
export const MyForm = Shade({
80+
shadowDomName: 'my-form',
81+
render: ({ useDisposable }) => {
82+
const handleSubmit = async () => {
83+
// This may fail due to shadow DOM encapsulation
84+
const form = document.querySelector('form[data-my-form]') as HTMLFormElement
85+
form?.reset()
86+
}
87+
88+
return <form data-my-form>{/* Form content */}</form>
89+
},
90+
})
91+
92+
// ✅ Good - use element from Shades context
93+
export const MyForm = Shade({
94+
shadowDomName: 'my-form',
95+
render: ({ element, useDisposable }) => {
96+
const handleSubmit = async () => {
97+
// Use element.querySelector to query within component's shadow DOM
98+
const form = element.querySelector<HTMLFormElement>('form[data-my-form]')
99+
form?.reset()
100+
}
101+
102+
return <form data-my-form>{/* Form content */}</form>
103+
},
104+
})
105+
106+
// ✅ Better - use state to trigger re-render instead of DOM manipulation
107+
export const MyForm = Shade({
108+
shadowDomName: 'my-form',
109+
render: ({ useDisposable, useObservable }) => {
110+
const formKey = useDisposable('formKey', () => new ObservableValue(0))
111+
const [key] = useObservable('formKeyValue', formKey)
112+
113+
const handleSubmit = async () => {
114+
// Success - increment key to force form re-render with fresh state
115+
formKey.setValue(key + 1)
116+
}
117+
118+
return <form key={key}>{/* Form content with fresh state */}</form>
119+
},
120+
})
121+
```
122+
123+
### Accessing DOM Elements in Components
124+
125+
When you need to access DOM elements within a Shades component:
126+
127+
1. **Use `element` from render context** - This is the component's shadow root
128+
2. **Use `element.querySelector()`** - Queries within the shadow DOM
129+
3. **Prefer state-driven updates** - Use ObservableValue to trigger re-renders instead of direct DOM manipulation
130+
131+
```typescript
132+
// ✅ Good - using element context for input focus
133+
export const SearchInput = Shade({
134+
shadowDomName: 'search-input',
135+
constructed: ({ element }) => {
136+
// Safe to use element.querySelector after construction
137+
element.querySelector<HTMLInputElement>('input[autofocus]')?.focus()
138+
},
139+
render: ({ element }) => {
140+
const focusInput = () => {
141+
element.querySelector<HTMLInputElement>('input')?.focus()
142+
}
143+
144+
return (
145+
<div>
146+
<input type="text" autofocus />
147+
<button onclick={focusInput}>Focus</button>
148+
</div>
149+
)
150+
},
151+
})
152+
```
153+
71154
## Form Patterns
72155

73156
### Standard Form Structure

.cursor/rules/OBSERVABLE_STATE.md

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,71 @@ export const Dashboard = Shade({
128128

129129
## useDisposable Hook
130130

131+
### Critical: Pairing useDisposable with useObservable
132+
133+
**When you need the component to re-render when an observable value changes**, you must subscribe to it with `useObservable`. Using `getValue()` directly in render will NOT trigger re-renders.
134+
135+
```typescript
136+
// ✅ Good - component re-renders when value changes
137+
export const FormComponent = Shade({
138+
shadowDomName: 'form-component',
139+
render: ({ useDisposable, useObservable }) => {
140+
// Create the observable with useDisposable
141+
const isLoadingObs = useDisposable('isLoading', () => new ObservableValue(false))
142+
const errorObs = useDisposable('error', () => new ObservableValue<string>(''))
143+
144+
// Subscribe with useObservable to trigger re-renders
145+
const [isLoading] = useObservable('isLoadingValue', isLoadingObs)
146+
const [error] = useObservable('errorValue', errorObs)
147+
148+
const handleSubmit = async () => {
149+
isLoadingObs.setValue(true)
150+
try {
151+
await doSomething()
152+
} catch (err) {
153+
errorObs.setValue(err instanceof Error ? err.message : 'Failed')
154+
} finally {
155+
isLoadingObs.setValue(false)
156+
}
157+
}
158+
159+
return (
160+
<div>
161+
{error && <div style={{ color: 'red' }}>{error}</div>}
162+
<button disabled={isLoading} onclick={() => void handleSubmit()}>
163+
{isLoading ? 'Loading...' : 'Submit'}
164+
</button>
165+
</div>
166+
)
167+
},
168+
})
169+
170+
// ❌ Bad - component won't re-render when values change!
171+
export const BrokenFormComponent = Shade({
172+
shadowDomName: 'broken-form',
173+
render: ({ useDisposable }) => {
174+
const isLoading = useDisposable('isLoading', () => new ObservableValue(false))
175+
const error = useDisposable('error', () => new ObservableValue<string>(''))
176+
177+
// These getValue() calls only get current value at render time
178+
// They do NOT subscribe, so changes won't trigger re-renders!
179+
return (
180+
<div>
181+
{error.getValue() && <div>{error.getValue()}</div>}
182+
<button disabled={isLoading.getValue()}>
183+
{isLoading.getValue() ? 'Loading...' : 'Submit'}
184+
</button>
185+
</div>
186+
)
187+
},
188+
})
189+
```
190+
191+
**Rule of thumb:**
192+
193+
- Use `useDisposable` alone when you only need to **set** values (e.g., in event handlers)
194+
- Pair with `useObservable` when you need the UI to **react** to value changes
195+
131196
### Local Component State
132197

133198
Use `useDisposable` for component-local reactive state:
@@ -499,6 +564,118 @@ export const LoginForm = Shade({
499564
});
500565
```
501566

567+
## Service Initialization and Disposal
568+
569+
### Services with Event Listeners
570+
571+
When services need to add event listeners (e.g., WebSocket messages), implement proper `init()` and `dispose()` methods:
572+
573+
```typescript
574+
// ✅ Good - service with init/dispose pattern
575+
@Injectable({ lifetime: 'singleton' })
576+
export class LoggingService {
577+
@Injected(WebsocketNotificationsService)
578+
declare readonly websocketNotificationsService: WebsocketNotificationsService
579+
580+
private logEntryCache = new Cache({
581+
capacity: 100,
582+
load: async (id: string) => {
583+
// Load logic
584+
},
585+
})
586+
587+
// Bind the handler to preserve 'this' context
588+
private onMessage = ((messageData: WebsocketMessage) => {
589+
if (messageData.type === 'log-entry-added') {
590+
this.logEntryCache.setExplicitValue({
591+
loadArgs: [messageData.logEntry.id],
592+
value: { status: 'loaded', value: messageData.logEntry, updatedAt: new Date() },
593+
})
594+
}
595+
}).bind(this)
596+
597+
public init() {
598+
this.websocketNotificationsService.addListener('onMessage', this.onMessage)
599+
}
600+
601+
public dispose() {
602+
this.websocketNotificationsService.removeListener('onMessage', this.onMessage)
603+
}
604+
605+
// Implement Symbol.dispose for automatic cleanup
606+
public [Symbol.dispose]() {
607+
this.dispose()
608+
}
609+
}
610+
```
611+
612+
### Service Initialization Patterns
613+
614+
Services that require initialization should be initialized explicitly:
615+
616+
```typescript
617+
// ✅ Good - explicit service initialization
618+
// In app setup or component
619+
const loggingService = injector.getInstance(LoggingService)
620+
loggingService.init()
621+
622+
// Or use useDisposable for automatic disposal
623+
export const LogViewer = Shade({
624+
shadowDomName: 'log-viewer',
625+
render: ({ injector, useDisposable }) => {
626+
const loggingService = useDisposable('loggingService', () => {
627+
const service = injector.getInstance(LoggingService)
628+
service.init()
629+
return {
630+
service,
631+
[Symbol.dispose]: () => service.dispose(),
632+
}
633+
})
634+
635+
return <div>Log viewer content</div>
636+
},
637+
})
638+
```
639+
640+
### Avoiding Memory Leaks
641+
642+
Always ensure listeners are removed when the service is no longer needed:
643+
644+
```typescript
645+
// ❌ Bad - listener never removed
646+
@Injectable({ lifetime: 'singleton' })
647+
export class BrokenService {
648+
@Injected(EventService)
649+
declare readonly eventService: EventService
650+
651+
constructor() {
652+
// Memory leak: listener added but never removed
653+
this.eventService.addListener('event', (data) => {
654+
this.handleEvent(data)
655+
})
656+
}
657+
}
658+
659+
// ✅ Good - listener properly managed
660+
@Injectable({ lifetime: 'singleton' })
661+
export class ProperService {
662+
@Injected(EventService)
663+
declare readonly eventService: EventService
664+
665+
private handleEvent = ((data: EventData) => {
666+
// Handle event
667+
}).bind(this)
668+
669+
public init() {
670+
this.eventService.addListener('event', this.handleEvent)
671+
}
672+
673+
public [Symbol.dispose]() {
674+
this.eventService.removeListener('event', this.handleEvent)
675+
}
676+
}
677+
```
678+
502679
## Summary
503680

504681
**Key Principles:**
@@ -513,6 +690,7 @@ export const LoginForm = Shade({
513690
8. **Manual subscriptions** must be disposed manually
514691
9. **Computed observables** for derived state
515692
10. **Symbol.dispose** for resource cleanup
693+
11. **init/dispose pattern** for services with event listeners
516694

517695
**Observable State Checklist:**
518696

@@ -525,6 +703,8 @@ export const LoginForm = Shade({
525703
- [ ] Symbol.dispose for service cleanup
526704
- [ ] No memory leaks (subscriptions cleaned up)
527705
- [ ] Type-safe observables
706+
- [ ] Services with listeners have init/dispose methods
707+
- [ ] Event handlers are bound to preserve `this` context
528708

529709
**Common Patterns:**
530710

0 commit comments

Comments
 (0)