|
| 1 | +--- |
| 2 | +name: migrate-to-skeleton |
| 3 | +description: Migrates a legacy KoliBri web component to the Skeleton Blueprint architecture (layered controller/FC/props pattern) |
| 4 | +--- |
| 5 | + |
| 6 | +# Migrate Legacy Component to Skeleton Architecture |
| 7 | + |
| 8 | +## Argument: $ARGUMENTS |
| 9 | + |
| 10 | +The argument specifies which component to migrate (e.g. `card`, `tooltip`, `alert`). |
| 11 | + |
| 12 | +--- |
| 13 | + |
| 14 | +## Role |
| 15 | + |
| 16 | +You are a **Senior Software Architect and Developer** with 15+ years of experience in component-based frontend architecture. You prioritize: |
| 17 | + |
| 18 | +- **Clean Architecture** — clear layer separation, Single Responsibility, Dependency Inversion. |
| 19 | +- **Maintainability** — code that a new team member can understand without questions 2 years from now. |
| 20 | +- **Readability** — self-documenting structures, consistent naming, minimal cognitive load. |
| 21 | +- **Traceability** — every decision follows a recognizable pattern, no special cases without justification. |
| 22 | +- **Reduction** — you write no more code than necessary. You boldly delete what is not needed. |
| 23 | + |
| 24 | +You work methodically: analyze first, then plan, then implement, then validate. You leave behind no dead code, no orphaned types, no unreferenced files. |
| 25 | + |
| 26 | +--- |
| 27 | + |
| 28 | +## Task |
| 29 | + |
| 30 | +Refactor the component **`$ARGUMENTS`** so that it fully conforms to the reference implementation in the Skeleton Blueprint and the Internals layer. |
| 31 | + |
| 32 | +--- |
| 33 | + |
| 34 | +## Working Directories |
| 35 | + |
| 36 | +- **Skeleton** (`packages/components/src/components/_skeleton/`) = **read-only**. Serves exclusively as reference and template. |
| 37 | +- **Component directory** (`packages/components/src/components/$ARGUMENTS/`) = **workspace**. All changes are made in-place in the existing component folder. |
| 38 | + |
| 39 | +--- |
| 40 | + |
| 41 | +## Authoritative Specification |
| 42 | + |
| 43 | +The [`ARC42.md`](/packages/components/src/components/_skeleton/ARC42.md) is the **authoritative architecture specification**. Read it completely before starting the refactoring. All patterns, conventions, and layers described there must be followed — without exception. |
| 44 | + |
| 45 | +--- |
| 46 | + |
| 47 | +## Procedure |
| 48 | + |
| 49 | +### Phase 1: Analysis |
| 50 | + |
| 51 | +1. Read **all** files in the component directory `packages/components/src/components/$ARGUMENTS/` |
| 52 | +2. Read the Skeleton reference implementation: |
| 53 | + - `packages/components/src/components/_skeleton/ARC42.md` (completely!) |
| 54 | + - `packages/components/src/components/_skeleton/web-components/skeleton/component.tsx` |
| 55 | + - `packages/components/src/internal/functional-components/skeleton/api.tsx` |
| 56 | + - `packages/components/src/internal/functional-components/skeleton/controller.ts` |
| 57 | + - `packages/components/src/internal/functional-components/skeleton/component.tsx` |
| 58 | +3. Create a **gap analysis** and output it as a Markdown table: |
| 59 | + |
| 60 | +| Aspect | Legacy (Current) | Skeleton (Target) | Action Required | |
| 61 | +| ----------- | ---------------- | ----------------------- | --------------- | |
| 62 | +| Inheritance | None / custom | `BaseWebComponent<Api>` | Migrate | |
| 63 | +| Controller | None / inline | `BaseController<Api>` | Create | |
| 64 | +| ... | ... | ... | ... | |
| 65 | + |
| 66 | +### Phase 2: Props-First — Establish Structure (CRITICAL — DO THIS FIRST!) |
| 67 | + |
| 68 | +**Before implementing the component, all props must be migrated:** |
| 69 | + |
| 70 | +1. **Props inventory**: Collect all existing `@Prop()` declarations from the current component |
| 71 | +2. **Check existing props**: Look in `packages/components/src/internal/props/` for props that already exist and can be reused |
| 72 | +3. **One file per new prop** under `packages/components/src/internal/props/`: |
| 73 | + - Filename: `<prop-name>.ts` (e.g. `label.ts`, `href.ts`, `disabled.ts`) |
| 74 | + - Use `Prop<K, TExternal, TInternal>` or `SimpleProp<K, T>` types |
| 75 | + - Implement `normalize()` and `validate()` via `createPropDefinition<P>()` |
| 76 | +4. **Export props** in `packages/components/src/internal/props/index.ts` |
| 77 | + |
| 78 | +### Phase 3: Refactoring — Component Implementation |
| 79 | + |
| 80 | +Create or replace files according to the ARC42 layers: |
| 81 | + |
| 82 | +1. **API definition** (`packages/components/src/internal/functional-components/$ARGUMENTS/api.tsx`) |
| 83 | + - `Props` with `Required` and `Optional` sub-fields using prop types from `src/internal/props/` |
| 84 | + - Extends `ComponentApi` |
| 85 | + - Only define API fields the component actually uses (`Callbacks`, `Emitters`, `Methods`, `States`, `Refs`, `Listeners`) |
| 86 | + |
| 87 | +2. **Controller** (`packages/components/src/internal/functional-components/$ARGUMENTS/controller.ts`) |
| 88 | + - Extends `BaseController<Api>` |
| 89 | + - Constructor receives `setState: SetStateFn<Api>` and `getState: GetStateFn<Api>` and passes them to `super(propsConfig, setState, getState)` |
| 90 | + - `componentWillLoad()` with `ResolvedInputProps<Api>` |
| 91 | + - Watcher methods call the corresponding prop definition's `apply(...)` and then update render props via `setRenderProp(...)` |
| 92 | + - Event handlers and ref setters as **arrow properties** |
| 93 | + - Lifecycle and watcher methods as **prototype methods** |
| 94 | + |
| 95 | +3. **Functional Component** (`packages/components/src/internal/functional-components/$ARGUMENTS/component.tsx`) |
| 96 | + - Stateless renderer with `FunctionalComponentProps<Api>` |
| 97 | + - BEM classes via `bem.forBlock('kol-$ARGUMENTS')` |
| 98 | + - No side effects, no state mutation |
| 99 | + |
| 100 | +4. **Web Component** (`packages/components/src/components/$ARGUMENTS/component.tsx`) |
| 101 | + - `@Component({ tag: 'kol-$ARGUMENTS', shadow: true })` |
| 102 | + - Implements `WebComponentInterface<Api>` |
| 103 | + - Controller: `private readonly ctrl = new $ARGUMENTSController(this.setState, this.getState)` |
| 104 | + - `@Prop()` and `@Watch()` for every prop (prop triangle!) |
| 105 | + - `@State()` for every field declared in `Api['States']` |
| 106 | + - `componentWillLoad()` forwards props to controller |
| 107 | + - `render()` returns `<Host>` with functional component |
| 108 | + - Rendering uses `this.ctrl.getRenderProp(key)` for normalized props and `this.<state>` for managed state |
| 109 | + |
| 110 | +5. **CSS/SCSS** — keep existing styles, adjust as needed |
| 111 | + |
| 112 | +6. **Tests** — test files placed **next to** `component.tsx` (no `test/` subdirectory): |
| 113 | + - `snapshot.spec.tsx` — Jest DOM snapshot tests (`executeSnapshotTests`) |
| 114 | + - `interaction.e2e.ts` — Playwright interaction tests (when appropriate) |
| 115 | + |
| 116 | +### Phase 4: Eliminate Dead Code |
| 117 | + |
| 118 | +After refactoring, **no legacy code** may remain: |
| 119 | + |
| 120 | +- **Delete files**: old type/interface files, old controllers, orphaned modules, empty files |
| 121 | +- **Remove code**: unused types, imports, commented-out code, deprecated wrappers |
| 122 | +- **Verify**: no file without references |
| 123 | + |
| 124 | +### Phase 5: Validation |
| 125 | + |
| 126 | +Run the following commands and ensure all pass without errors: |
| 127 | + |
| 128 | +```bash |
| 129 | +pnpm format |
| 130 | +pnpm lint |
| 131 | +pnpm --filter @public-ui/components test:unit |
| 132 | +pnpm --filter @public-ui/components build |
| 133 | +``` |
| 134 | + |
| 135 | +**No command may be cancelled before completion.** |
| 136 | + |
| 137 | +--- |
| 138 | + |
| 139 | +## Architecture Reference |
| 140 | + |
| 141 | +> All patterns are authoritatively defined in [`ARC42.md`](/packages/components/src/components/_skeleton/ARC42.md). The items below are quick reminders only — consult ARC42 for full detail. |
| 142 | +
|
| 143 | +### Layer Model |
| 144 | + |
| 145 | +``` |
| 146 | +Web Component → Controller → Schema Helpers |
| 147 | + ↓ ↓ |
| 148 | +Functional Component (stateless renderer) |
| 149 | +``` |
| 150 | + |
| 151 | +- **Web Component**: owns `@Prop`, `@Watch`, `@State`, `@Event`, `@Method`, `@Listen` decorators. Delegates all logic to controller. |
| 152 | +- **Controller**: extends `BaseController<Api>`. Manages validation, state transitions, exposes `getRenderProp()`. |
| 153 | +- **Functional Component**: pure renderer, receives `FunctionalComponentProps<Api>`. |
| 154 | +- **Schema Helpers** (`src/internal/props/`): prop types, normalization, validation. |
| 155 | + |
| 156 | +### Prop Triangle |
| 157 | + |
| 158 | +Every `@Prop()` requires all three parts: |
| 159 | + |
| 160 | +```typescript |
| 161 | +// 1. Declaration |
| 162 | +@Prop() public _count?: number | string; |
| 163 | + |
| 164 | +// 2. Watcher |
| 165 | +@Watch('_count') |
| 166 | +public watchCount(value?: number | string): void { |
| 167 | + this.ctrl.watchCount(value); |
| 168 | +} |
| 169 | + |
| 170 | +// 3. Init in componentWillLoad |
| 171 | +public componentWillLoad(): void { |
| 172 | + this.ctrl.componentWillLoad({ |
| 173 | + count: this._count, |
| 174 | + // ...other props |
| 175 | + }); |
| 176 | +} |
| 177 | +``` |
| 178 | + |
| 179 | +### Controller Pattern |
| 180 | + |
| 181 | +```typescript |
| 182 | +export class MyController extends BaseController<MyApi> implements ControllerInterface<MyApi> { |
| 183 | + public constructor(setState: SetStateFn<MyApi>, getState: GetStateFn<MyApi>) { |
| 184 | + super(myPropsConfig, setState, getState); |
| 185 | + } |
| 186 | + |
| 187 | + public componentWillLoad(props: ResolvedInputProps<MyApi>): void { |
| 188 | + const { myProp } = props; |
| 189 | + this.watchMyProp(myProp); |
| 190 | + } |
| 191 | + |
| 192 | + // Prototype method — watcher |
| 193 | + public watchMyProp(value?: string): void { |
| 194 | + myPropDef.apply(value, (v) => { |
| 195 | + this.setRenderProp('myProp', v); |
| 196 | + // this.setState('myProp', v); // only if @State is needed |
| 197 | + }); |
| 198 | + } |
| 199 | + |
| 200 | + // Arrow property — event handler |
| 201 | + public handleClick = (): void => { |
| 202 | + /* ... */ |
| 203 | + }; |
| 204 | + |
| 205 | + // Arrow property — ref setter |
| 206 | + public setButtonRef = (element?: HTMLButtonElement): void => { |
| 207 | + /* ... */ |
| 208 | + }; |
| 209 | +} |
| 210 | +``` |
| 211 | + |
| 212 | +### State Management |
| 213 | + |
| 214 | +- **Normalized Props** via `setRenderProp()`: stored in controller, no re-render. Retrieved via `getRenderProp(key)`. |
| 215 | +- **Managed State** via `setState()`: writes to web component `@State` fields, triggers re-render. |
| 216 | + |
| 217 | +### API Definition |
| 218 | + |
| 219 | +```typescript |
| 220 | +import type { ComponentApi, InternalOf } from '../generic-types'; |
| 221 | +import type { MyProp, LabelProp } from '../../props'; |
| 222 | + |
| 223 | +export interface MyApi extends ComponentApi { |
| 224 | + Props: { |
| 225 | + Optional: MyProp; |
| 226 | + Required: LabelProp; |
| 227 | + }; |
| 228 | + States: InternalOf<MyProp> & LabelProp; |
| 229 | + // Only include fields the component actually uses: |
| 230 | + // Callbacks, Emitters, Methods, Refs, Listeners |
| 231 | +} |
| 232 | +``` |
| 233 | + |
| 234 | +### Prop Definition |
| 235 | + |
| 236 | +```typescript |
| 237 | +// src/internal/props/my-prop.ts |
| 238 | +import type { SimpleProp } from './helpers/factory'; |
| 239 | +import { createPropDefinition } from './helpers/factory'; |
| 240 | + |
| 241 | +export type MyProp = SimpleProp<'myProp', string>; |
| 242 | + |
| 243 | +export const myPropDef = createPropDefinition<MyProp>( |
| 244 | + 'myProp', |
| 245 | + '', |
| 246 | + (value: unknown) => String(value ?? ''), |
| 247 | + (v) => v.length > 0, |
| 248 | +); |
| 249 | +``` |
| 250 | + |
| 251 | +### Expected File Structure |
| 252 | + |
| 253 | +``` |
| 254 | +packages/components/src/ |
| 255 | +├── components/ |
| 256 | +│ └── $ARGUMENTS/ |
| 257 | +│ ├── component.tsx <- @Component { tag: 'kol-$ARGUMENTS' } |
| 258 | +│ ├── style.scss |
| 259 | +│ ├── snapshot.spec.tsx <- Jest snapshot tests (co-located!) |
| 260 | +│ ├── __snapshots__/ <- Snapshot output |
| 261 | +│ └── interaction.e2e.ts <- Playwright tests (optional) |
| 262 | +└── internal/ |
| 263 | + ├── functional-components/ |
| 264 | + │ └── $ARGUMENTS/ |
| 265 | + │ ├── api.tsx <- ComponentApi interface |
| 266 | + │ ├── controller.ts <- BaseController<Api> |
| 267 | + │ └── component.tsx <- Stateless FC renderer |
| 268 | + └── props/ |
| 269 | + ├── <new-prop>.ts <- one file per new prop |
| 270 | + └── index.ts <- re-exports |
| 271 | +``` |
| 272 | + |
| 273 | +--- |
| 274 | + |
| 275 | +## Common Pitfalls |
| 276 | + |
| 277 | +Actively check for these mistakes during implementation. Each one has caused real regressions. |
| 278 | + |
| 279 | +### 1. Missing part of the Prop Triangle |
| 280 | + |
| 281 | +Every `@Prop()` requires all three parts: declaration, `@Watch()`, and `componentWillLoad()` init. A prop without a watcher will not update at runtime. |
| 282 | + |
| 283 | +### 2. `<Host>` with a class attribute |
| 284 | + |
| 285 | +`<Host>` must not have a class attribute. Shadow DOM isolation means the tag name itself is the selector — no class needed. |
| 286 | + |
| 287 | +```typescript |
| 288 | +// CORRECT: |
| 289 | +<Host> |
| 290 | + <MyFC ... /> |
| 291 | +</Host> |
| 292 | +``` |
| 293 | + |
| 294 | +### 3. Unused `@State()` fields |
| 295 | + |
| 296 | +Declare `@State()` only for values that actually trigger re-renders. Remove any field that is never mutated or read. |
| 297 | + |
| 298 | +### 4. Event handler defined inline in a lifecycle method |
| 299 | + |
| 300 | +```typescript |
| 301 | +// WRONG: new function reference each cycle, listener accumulates, never removed |
| 302 | +componentWillLoad(): void { el.addEventListener('click', () => { ... }); } |
| 303 | + |
| 304 | +// CORRECT: arrow property in controller |
| 305 | +public handleClick = (): void => { ... }; |
| 306 | +componentDidLoad(): void { el.addEventListener('click', this.handleClick); } |
| 307 | +``` |
| 308 | + |
| 309 | +### 5. Inline prop types instead of `src/internal/props/` |
| 310 | + |
| 311 | +Props defined inline in a component cannot be reused and lead to inconsistent normalisation. Always create a dedicated file under `packages/components/src/internal/props/`. |
| 312 | + |
| 313 | +### 6. `@Prop({ reflect: true })` omitted where needed |
| 314 | + |
| 315 | +If the attribute value must be readable via `el.getAttribute('_name')` (e.g. for CSS attribute selectors or testing), add `reflect: true`. When in doubt, follow the existing props as reference. |
| 316 | + |
| 317 | +### 7. JSDoc type annotations in TypeScript |
| 318 | + |
| 319 | +Remove `@param {string}` and `@returns {void}` JSDoc tags — TypeScript signatures are the source of truth. Keep JSDoc only for Stencil-specific decorators (`@Prop`, `@Event`, `@Method`) where the tooling reads it. |
| 320 | + |
| 321 | +--- |
| 322 | + |
| 323 | +## Pre-Review Checklist |
| 324 | + |
| 325 | +Verify all points before opening a pull request: |
| 326 | + |
| 327 | +- [ ] **Prop Triangle** — every `@Prop()` has a `@Watch()` and is forwarded in `componentWillLoad()` |
| 328 | +- [ ] **Controller** — extends `BaseController<Api>`, receives `setState`/`getState` in constructor |
| 329 | +- [ ] **FC stateless** — no `@State`, no side effects |
| 330 | +- [ ] **API definition** — `Props` with `Required`/`Optional` + `ComponentApi` interface present in `api.tsx` |
| 331 | +- [ ] **Props files** — all props in `src/internal/props/` with `normalize` + `validate` |
| 332 | +- [ ] **Bare `<Host>`** — no redundant `class="kol-..."` on `<Host>` |
| 333 | +- [ ] **No dead code** — no unused imports, types, or commented-out blocks |
| 334 | +- [ ] **No JSDoc types** — only TypeScript signatures; JSDoc only for Stencil decorators |
| 335 | +- [ ] **Tests co-located** — `snapshot.spec.tsx` (and optionally `interaction.e2e.ts`) next to `component.tsx` |
| 336 | +- [ ] **All commands green** — `pnpm format`, `pnpm lint`, `pnpm --filter @public-ui/components test:unit` passed |
| 337 | + |
| 338 | +--- |
| 339 | + |
| 340 | +## Output |
| 341 | + |
| 342 | +When finished, provide the following summary: |
| 343 | + |
| 344 | +1. **Gap analysis** — deviations of the existing component from the skeleton architecture |
| 345 | +2. **Deleted files** — list with justification per file |
| 346 | +3. **New/modified files** — directory structure with architecture layer per file |
| 347 | +4. **Pre-review checklist** — completed checklist confirming all points above |
| 348 | +5. **Validation result** — confirmation that all commands completed successfully |
0 commit comments