docs(ai-docs): add patterns documentation - TypeScript, React, MobX, Web Components, Testing #591
Conversation
…Web Components, Testing
- Remove 'Files Analyzed' sections - Remove 'Anti-Patterns Found' sections - Remove analysis/summary paragraphs - Make all guidance prescriptive (MUST, ALWAYS, NEVER) - Focus on actionable patterns for LLM code generation
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
Shreyas281299
changed the title
| - **MUST** encapsulate business logic in custom hooks (`helper.ts`) | ||
| - **NEVER** access store directly in presentational components | ||
| - **NEVER** call SDK methods directly in components (use hooks) | ||
| - **NEVER** use class components |
There was a problem hiding this comment.
Overall the guidelines are solid, but the rule list would benefit from some re-organization for clarity and consistency. Several related rules are currently scattered (e.g., functional vs class components, store access, business logic in hooks, SDK usage), and some constraints are repeated using both MUST and NEVER forms to express the same idea. Consider grouping related rules together, consolidating overlapping bullets into a single clear statement, and aligning individual rules more explicitly with the “Widget → Hook → Component” pattern. This would reduce redundancy, improve readability, and make the architectural intent easier to understand and follow.
| │ Presentational Component │ ← Pure UI, props only | ||
| │ packages/cc-components/src/... │ | ||
| └─────────────────────────────────────┘ | ||
| ``` |
There was a problem hiding this comment.
The patterns are correct, but the documentation order isn’t aligned with the “Three-Layer Architecture” sequence, which makes the flow harder to follow. Since the diagram defines Widget → Hook → Presentational Component (with ErrorBoundary/observer as part of the Widget layer), it would read more clearly if the sections were reorganized to match that order—e.g., start with Widget Pattern (including ErrorBoundary/observer wrapper), then Custom Hook Pattern, then Presentational Component Pattern (and keep ErrorBoundary either embedded in the Widget section or placed immediately next to it).
| - **MUST** place unit tests in `tests/` folder within each package | ||
| - **MUST** place E2E tests in `playwright/` folder at repo root | ||
| - **NEVER** test implementation details - test behavior | ||
| - **NEVER** use CSS selectors in tests - use `data-testid` |
There was a problem hiding this comment.
The rules say “NEVER use CSS selectors – use data-testid”, but the Playwright utils include waitForSelector('[data-testid="login-success"]') (still a CSS selector). To align with the rule and reduce confusion, consider using await expect(page.getByTestId('login-success')).toBeVisible() or await page.getByTestId('login-success').waitFor() instead.
| @observable agentId: string = ''; | ||
| @observable currentState: string = ''; | ||
| @observable idleCodes: IdleCode[] = []; | ||
| @observable isLoggedIn: boolean = false; |
There was a problem hiding this comment.
Incorrect MobX decorator pattern
Issue: Documentation shows decorator-based MobX patterns (@observable, @action, @computed), but the actual codebase uses makeAutoObservable() without decorators.
Current (WRONG):
import { observable, makeObservable } from 'mobx';
class Store {
@observable agentId: string = '';
@observable teams: Team[] = [];
constructor() {
makeObservable(this);
}
}
Actual Implementation (packages/contact-center/store/src/store.ts:54-58):
class Store implements IStore {
teams: Team[] = [];
agentId: string = '';
// NO DECORATORS!
constructor() {
makeAutoObservable(this, {
cc: observable.ref,
});
}
}
Required Fix:
Replace the entire "Observable Decorator Pattern" section with:
makeAutoObservable Pattern
ALWAYS use makeAutoObservable for store classes:
import { makeAutoObservable, observable } from 'mobx';
class Store {
// Plain property declarations (no decorators)
agentId: string = '';
teams: Team[] = [];
currentState: string = '';
isLoggedIn: boolean = false;
constructor() {
// makeAutoObservable automatically makes properties observable
makeAutoObservable(this, {
// Only specify overrides for special cases
cc: observable.ref, // Don't observe nested properties
});
}
}
| // tests/{widget}/index.test.tsx | ||
| import { render, screen, fireEvent, waitFor } from '@testing-library/react'; | ||
| import { UserState } from '../../src/user-state'; | ||
| import { mockStore } from '@webex/cc-test-fixtures'; |
There was a problem hiding this comment.
Incorrect mock store pattern
Documentation shows mocking from @webex/cc-test-fixtures, but actual tests use @webex/test-fixtures and mock individual properties.
Current (WRONG):
import { mockStore } from '@webex/cc-test-fixtures';
jest.mock('@webex/cc-store', () => ({
__esModule: true,
default: mockStore,
}));
Actual Pattern (cc-components/tests/components/UserState/user-state.tsx:7):
import {mockCC} from '@webex/test-fixtures';
const defaultProps: IUserState = {
cc: mockCC,
agentId: 'agent123',
idleCodes: [...],
// ... other props
};
Suggestion:
Replace section with:
// Mock individual properties for component tests
import {mockCC} from '@webex/test-fixtures';
const mockLogger = {
log: jest.fn(),
info: jest.fn(),
warn: jest.fn(),
error: jest.fn(),
};
const defaultProps: IUserState = {
cc: mockCC,
agentId: 'test-agent-123',
idleCodes: [
{id: '0', name: 'Available', isSystem: true, isDefault: false},
],
logger: mockLogger,
// ... other required props
};
render(<UserStateComponent {...defaultProps} />);
| --- | ||
|
|
||
| ## Playwright E2E Test Pattern | ||
|
|
There was a problem hiding this comment.
Add TestManager pattern to Playwright section
Issue:
Example doesn't match actual test structure which uses TestManager class.
Actual Pattern (playwright/tests/station-login-test.spec.ts:14-28):
test.describe('Station Login Tests', () => {
let testManager: TestManager;
test.beforeAll(async ({browser}, testInfo) => {
const projectName = testInfo.project.name;
testManager = new TestManager(projectName);
await testManager.setupForStationLogin(browser);
});
test.afterAll(async () => {
if (testManager) {
await testManager.cleanup();
}
});
});
Suggestion:
Add before the Playwright Utils Pattern section:
| return this.tasks.length; | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Remove unused computed pattern
Issue:
Documentation shows @computed pattern, but it's not used anywhere in the actual store.
Suggestion:
Either:
- Remove this section entirely, OR
- Add a note:
Computed Pattern
akulakum
left a comment
akulakum
left a comment
There was a problem hiding this comment.
Add verification metadata to all files
Issue:
No way to track when documentation was last verified against the codebase.
Suggestion:
Add to the top of each file (after title, before Rules):
Last Verified: 2026-01-16
Verified Against: @webex/cc-widgets
Codebase Version: Current main branch
Status: ✅ Accurate / ⚠️ Needs Update
2 participants
COMPLETES N/A - Documentation Enhancement
This pull request addresses
Adding comprehensive pattern documentation to guide AI assistants and developers on the coding standards and patterns used in the Contact Center Widgets codebase.
by making the following changes
ai-docs/patterns/typescript-patterns.md- TypeScript conventions, type definitions, and best practicesai-docs/patterns/react-patterns.md- React component patterns, hooks, and lifecycle managementai-docs/patterns/mobx-patterns.md- MobX state management patterns with observer HOCai-docs/patterns/web-component-patterns.md- r2wc patterns for Web Component wrappersai-docs/patterns/testing-patterns.md- Jest and Playwright testing patternsChange Type
The following scenarios were tested
The GAI Coding Policy And Copyright Annotation Best Practices
Checklist before merging
Make sure to have followed the contributing guidelines before submitting.