Skip to content

Commit 408710e

Browse files
committed
command updates, prettier fixes
1 parent 5e92ccb commit 408710e

File tree

5 files changed

+349
-2
lines changed

5 files changed

+349
-2
lines changed

.cursor/commands/review-changes.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ For each issue, be specific: file, line, problem, suggested fix.
6767
**5. Learnings & Rule Updates:**
6868

6969
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+
7071
- Update an existing rule file in `.cursor/rules/`
7172
- Create a new rule file for the pattern
7273
- Skip persisting the learning

.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: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ export const BrokenFormComponent = Shade({
189189
```
190190

191191
**Rule of thumb:**
192+
192193
- Use `useDisposable` alone when you only need to **set** values (e.g., in event handlers)
193194
- Pair with `useObservable` when you need the UI to **react** to value changes
194195

@@ -563,6 +564,118 @@ export const LoginForm = Shade({
563564
});
564565
```
565566

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+
566679
## Summary
567680

568681
**Key Principles:**
@@ -577,6 +690,7 @@ export const LoginForm = Shade({
577690
8. **Manual subscriptions** must be disposed manually
578691
9. **Computed observables** for derived state
579692
10. **Symbol.dispose** for resource cleanup
693+
11. **init/dispose pattern** for services with event listeners
580694

581695
**Observable State Checklist:**
582696

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

593709
**Common Patterns:**
594710

.cursor/rules/TESTING_GUIDELINES.md

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,155 @@ describe('DataService', () => {
375375
})
376376
```
377377

378+
## Backend Action Testing
379+
380+
### Testing REST Actions
381+
382+
Test backend actions with proper mocking of injector dependencies:
383+
384+
```typescript
385+
// ✅ Good - backend action unit test
386+
import { Injector } from '@furystack/inject'
387+
import { StoreManager } from '@furystack/core'
388+
import { describe, expect, it, vi } from 'vitest'
389+
import { RegisterAction } from './register-action.js'
390+
391+
describe('RegisterAction', () => {
392+
const createTestInjector = () => {
393+
const injector = new Injector()
394+
395+
// Mock store manager
396+
const mockUserStore = {
397+
get: vi.fn(),
398+
add: vi.fn(),
399+
remove: vi.fn(),
400+
}
401+
402+
const mockCredentialStore = {
403+
add: vi.fn(),
404+
}
405+
406+
injector.setExplicitInstance(
407+
{
408+
getStoreFor: vi.fn((model) => {
409+
if (model.name === 'User') return mockUserStore
410+
if (model.name === 'PasswordCredential') return mockCredentialStore
411+
throw new Error(`Unknown model: ${model.name}`)
412+
}),
413+
} as unknown as StoreManager,
414+
StoreManager,
415+
)
416+
417+
return { injector, mockUserStore, mockCredentialStore }
418+
}
419+
420+
it('should create a new user when username does not exist', async () => {
421+
const { injector, mockUserStore, mockCredentialStore } = createTestInjector()
422+
423+
mockUserStore.get.mockResolvedValue(null) // User doesn't exist
424+
mockUserStore.add.mockResolvedValue({ username: '[email protected]', roles: [] })
425+
mockCredentialStore.add.mockResolvedValue({})
426+
427+
const result = await RegisterAction({
428+
injector,
429+
getBody: async () => ({ username: '[email protected]', password: 'password123' }),
430+
response: {} as Response,
431+
request: {} as Request,
432+
})
433+
434+
expect(mockUserStore.add).toHaveBeenCalledWith(expect.objectContaining({ username: '[email protected]' }))
435+
})
436+
437+
it('should throw 409 when user already exists', async () => {
438+
const { injector, mockUserStore } = createTestInjector()
439+
440+
mockUserStore.get.mockResolvedValue({ username: '[email protected]' })
441+
442+
await expect(
443+
RegisterAction({
444+
injector,
445+
getBody: async () => ({ username: '[email protected]', password: 'password123' }),
446+
response: {} as Response,
447+
request: {} as Request,
448+
}),
449+
).rejects.toThrow('User already exists')
450+
})
451+
})
452+
```
453+
454+
### Action Test Patterns
455+
456+
When testing backend actions:
457+
458+
1. **Mock StoreManager** - Provide mock stores for each entity type
459+
2. **Mock Authentication** - Use `HttpUserContext` for authenticated actions
460+
3. **Test error cases** - Verify proper RequestError codes (400, 401, 404, 409, 500)
461+
4. **Test cleanup logic** - Verify partial state is cleaned up on failure
462+
463+
```typescript
464+
// ✅ Good - testing authenticated action
465+
import { getCurrentUser } from '@furystack/core'
466+
467+
vi.mock('@furystack/core', () => ({
468+
getCurrentUser: vi.fn(),
469+
}))
470+
471+
describe('PasswordResetAction', () => {
472+
it('should throw 401 when user is not authenticated', async () => {
473+
vi.mocked(getCurrentUser).mockResolvedValue(null)
474+
475+
await expect(PasswordResetAction({ injector, getBody: async () => ({}) })).rejects.toThrow('User not authenticated')
476+
})
477+
478+
it('should throw 400 when current password is incorrect', async () => {
479+
vi.mocked(getCurrentUser).mockResolvedValue({ username: '[email protected]' })
480+
mockAuthenticator.setPasswordForUser.mockRejectedValue(new UnauthenticatedError())
481+
482+
await expect(
483+
PasswordResetAction({
484+
injector,
485+
getBody: async () => ({ currentPassword: 'wrong', newPassword: 'new123' }),
486+
}),
487+
).rejects.toThrow('Current password is incorrect')
488+
})
489+
})
490+
```
491+
492+
### RequestError Testing
493+
494+
Test that actions throw appropriate HTTP error codes:
495+
496+
```typescript
497+
// ✅ Good - testing RequestError codes
498+
import { RequestError } from '@furystack/rest'
499+
500+
it('should throw 409 for duplicate resource', async () => {
501+
// Setup duplicate condition
502+
mockStore.find.mockResolvedValue({ count: 1 })
503+
504+
try {
505+
await CreateResourceAction({ injector, getBody: async () => ({}) })
506+
fail('Expected RequestError to be thrown')
507+
} catch (error) {
508+
expect(error).toBeInstanceOf(RequestError)
509+
expect((error as RequestError).responseCode).toBe(409)
510+
}
511+
})
512+
513+
it('should throw 400 for validation errors', async () => {
514+
try {
515+
await CreateResourceAction({
516+
injector,
517+
getBody: async () => ({ invalidField: true }),
518+
})
519+
fail('Expected RequestError to be thrown')
520+
} catch (error) {
521+
expect(error).toBeInstanceOf(RequestError)
522+
expect((error as RequestError).responseCode).toBe(400)
523+
}
524+
})
525+
```
526+
378527
## Test Organization
379528

380529
### Co-location

0 commit comments

Comments
 (0)