Merge pull request #243 from makeup/refactor

Refactor core modules + revamp all demo pages

Author: ianmcburnie

.claude/skills/add-unit-tests/SKILL.md

@@ -0,0 +1,72 @@
+---
+description: Write missing unit tests for a makeup-js module based on coverage gaps
+args: `<module-name>`
+---
+
+You are writing missing unit tests for a makeup-js module. Your goal is to fill coverage gaps with well-structured tests that follow the existing conventions — not to rewrite existing tests or pad coverage with low-value assertions.
+
+**Input**
+
+The user will provide a module name (e.g., "makeup-next-id" or full path like "packages/core/makeup-next-id"). Optionally, the user may paste a gap report from `/test-coverage` — if so, use it to skip re-running coverage and go straight to writing.
+
+**Process**
+
+**1. Locate the module**
+
+- Find the module in `packages/core/` or `packages/ui/`
+- Read `src/index.js` (or all files in `src/`)
+- Read `test/index.js` (or all files in `test/`)
+
+**2. Identify gaps**
+
+If the user did not provide a gap report, run coverage to find them:
+
+```
+npm run test:coverage -- --coverage.include="packages/core/<module-name>/src/**" packages/core/<module-name>
+```
+
+Read the source and tests side by side to identify:
+
+- Uncovered branches (`if`/`else`, ternaries, `?.`, `??`)
+- Uncovered functions
+- Missing edge cases (empty input, boundary values, error conditions)
+- Missing negative cases (invalid input that is guarded in code)
+
+Skip gaps that are not worth testing (non-deterministic internals, unreachable defensive guards, browser-only paths that jsdom cannot exercise).
+
+**3. Write the tests**
+
+Append the new tests to `test/index.js`. Do not modify existing tests.
+
+Follow the existing conventions exactly:
+
+- BDD structure: `describe("given X") > describe("when Y") > it("should Z")`
+- One assertion per `it` block where practical
+- Use `beforeEach`/`beforeAll` for setup, matching the pattern already in the file
+- Use `vi.spyOn()` for mocking, `vi.useFakeTimers()` for timers — restore in `afterEach`
+- Import only what is needed; do not add imports already present at the top of the file
+- Match the indentation and formatting of the existing file
+
+**4. Run and verify**
+
+Run the tests to confirm they pass:
+
+```
+npm run test:unit -- packages/core/<module-name>
+```
+
+If any test fails, fix it before proceeding. Do not leave failing tests.
+
+Then re-run coverage to confirm the gaps are closed:
+
+```
+npm run test:coverage -- --coverage.include="packages/core/<module-name>/src/**" packages/core/<module-name>
+```
+
+**5. Summary**
+
+Report:
+
+- Which gaps were addressed and which were intentionally skipped (and why)
+- Updated coverage numbers
+- Number of tests added

.claude/skills/refactor-module/SKILL.md

@@ -0,0 +1,147 @@
+---
+description: Systematically refactor a makeup-js module with modern JavaScript patterns
+args: <module-name>
+---
+
+You are refactoring a module in the makeup-js monorepo to use modern JavaScript features while maintaining readability and test compatibility.
+
+# Input
+
+The user will provide a module name (e.g., "makeup-next-id" or full path like "packages/core/makeup-next-id").
+
+# Process
+
+## 1. Locate and analyze the module
+
+- Find the module in `packages/core/` or `packages/ui/`
+- Read the README.md **first** — it describes intended behavior from the consumer's perspective and is the authoritative source for what the module should do. Use it to inform all refactoring decisions, especially around edge cases and event behavior.
+- Read the source file(s) in `src/`
+- Read the test file(s) in `test/`
+- Check the package.json for any specific configuration
+
+## 2. Analyze refactoring opportunities
+
+Identify modernization opportunities based on browserslist (modern browsers):
+
+**Prioritize readability over cleverness**
+
+Common patterns to modernize:
+
+- Replace deprecated `keyCode` with `key` property
+- Replace magic numbers with named constants
+- Use `const`/`let` consistently (no `var`)
+- Replace `Object.assign({}, obj1, obj2)` with spread: `{...obj1, ...obj2}` — rename the result only if the existing name is genuinely unclear; do not rename variables solely to remove a `_` prefix
+- Use arrow functions for callbacks and simple functions
+- Use `Set` or `Map` where appropriate for better performance/readability
+- Remove unnecessary `"use strict"` (not needed in ES modules)
+- Use template literals for string concatenation
+- Use optional chaining `?.` where it improves readability
+- Use nullish coalescing `??` for default values (prefer over `||` when appropriate)
+- Use `Array.from()`, `.forEach()`, `.map()`, `.filter()` etc. where readable
+- Do NOT simplify `=== true` / `=== false` comparisons — they carry type information that TypeScript would otherwise provide. Leave them as-is.
+
+**Avoid over-engineering:**
+
+- Don't use private fields (`#`) unless there's a clear encapsulation benefit
+- Don't restructure class architecture unnecessarily
+- Keep logic simple and straightforward
+
+**Preserve existing comments:**
+
+- Do NOT remove comments from code you are refactoring
+- Comments often explain non-obvious behavior, browser quirks, or intentional decisions that aren't evident from the code alone
+- Only remove a comment if it is purely restating what the code already clearly says (e.g. `i++ // increment i`)
+
+**Optimize for tree shaking:**
+
+- Prefer named exports over default exports where possible
+- Prefer named imports (`import { foo } from "pkg"`) over namespace imports (`import * as Pkg from "pkg"`) — named imports allow bundlers to tree-shake unused exports
+
+**CRITICAL - No API changes:**
+
+- Maintain existing API surface exactly
+- Do NOT change exported functions, classes, or methods
+- Do NOT change function signatures (parameters, return types)
+- Do NOT change event names or event detail structures
+- Do NOT introduce breaking changes
+- This is internal refactoring only - the public API must remain unchanged
+
+## 3. Explain your understanding and ask questions
+
+Before making any suggestions or changes, write a plain-text explanation of the module:
+
+- What the module does from a consumer's perspective (what problem it solves, how it is used)
+- What its key internal mechanisms are (how it works)
+- Any non-obvious design decisions you noticed (e.g. why certain patterns are used, what tradeoffs are made)
+- Any questions you have — about intent, edge cases, or anything in the source or tests that is unclear or surprising
+
+Then **wait for the user to respond** before proceeding. Do not begin refactoring until the user has confirmed your understanding is correct and answered any questions.
+
+## 5. Refactor tests FIRST
+
+**CRITICAL:** Maintain the given/when/then BDD test format.
+
+Test improvements:
+
+- Remove redundant import existence checks
+- Replace `keyCode` with `key` in KeyboardEvent tests
+- Use `vi.spyOn()` instead of manually mocking functions
+- Add specific test cases for edge cases
+- Use descriptive test names following given/when/then
+- Ensure proper cleanup in beforeEach/afterEach
+- Structure: `describe("given X") > describe("when Y") > it("should Z")`
+
+## 6. Refactor module code
+
+Before writing any code changes, explain each planned source change as a learning opportunity:
+
+- State what is changing and show the before/after
+- Explain _why_ the new form is preferred — the underlying principle, not just the rule (e.g. what problem does `Object.assign` have that spread solves? why does eliminating the intermediate variable improve readability?)
+- Connect the change to a broader pattern where relevant (e.g. "this is the same idea as destructuring — pulling out only what you need")
+
+Then apply all changes.
+
+## 7. Update README
+
+- Verify README is in sync with the refactored code
+- Check that:
+  - Usage examples are accurate (no API changes, so should still work)
+  - Code examples use correct variable names and syntax
+  - Examples are clear and follow current code
+- **Verify every listed property actually exists** — check each entry in the Properties section against the source. Look for getters, public instance fields, and properties set in the constructor. Remove or correct any that don't exist.
+- **Verify every listed method actually exists** — check each entry in the Methods section against the source. Look for class methods and exported functions.
+- **Verify every listed event name matches a `dispatchEvent` call** — search the source for `new CustomEvent(...)` and confirm the names match what the Events section documents.
+- **Verify every listed option is present in `defaultOptions`** — confirm each option key in the Options section exists in the module's default options object.
+- Fix any typos or inconsistencies in examples
+- Do NOT change the API in the README - this is verification only
+- Remove the "Dependencies" section if present — it duplicates package.json and drifts out of sync
+- Keep documentation concise and accurate
+
+## 8. Compile and test
+
+- Run `npm run compile --workspace=<module-name>`
+- Run `npm run test:unit -- packages/core/<module-name>` or `packages/ui/<module-name>`
+- Verify all tests pass
+- If tests fail, analyze and fix
+
+## 9. Summary
+
+Provide a concise summary:
+
+- Module name and location
+- Test improvements made
+- Code improvements made
+- Test results
+- Lines of code before/after (if significantly different)
+
+# Browser Support
+
+Target modern browsers per @ebay/browserslist-config. All modern ES features are supported.
+
+# Testing
+
+Tests use Vitest. Maintain compatibility with existing test infrastructure.
+
+# Output Format
+
+Be concise. Show what was changed and why. Confirm tests pass.