From 45cfd56b2c4e40f863b85987621c909318cc3c30 Mon Sep 17 00:00:00 2001 From: hyochan Date: Fri, 13 Mar 2026 16:18:44 +0900 Subject: [PATCH 1/3] fix: handle errors in css.__withStyles getDynamicPatch Add try-catch around getDynamicPatch({}) and JSON.stringify in __withStyles to prevent crashes when: - getDynamicPatch contains theme-dependent functions (calling with {} has no theme) - Dynamic patch objects contain circular references (e.g., DOM elements on web) Co-Authored-By: Claude Opus 4.6 --- .../kstyled/src/__tests__/runtime.test.tsx | 37 ++++++++++++++++ packages/kstyled/src/css.tsx | 44 ++++++++++++------- 2 files changed, 65 insertions(+), 16 deletions(-) diff --git a/packages/kstyled/src/__tests__/runtime.test.tsx b/packages/kstyled/src/__tests__/runtime.test.tsx index 8cdbea3..e15e705 100644 --- a/packages/kstyled/src/__tests__/runtime.test.tsx +++ b/packages/kstyled/src/__tests__/runtime.test.tsx @@ -426,6 +426,43 @@ describe('kstyled Runtime Tests', () => { expect(patch.marginTop).toBe(32); expect(patch.backgroundColor).toBe('#007AFF'); }); + + test('should not crash when getDynamicPatch throws (theme-dependent css``)', () => { + // Simulates compiled css`` with theme interpolation: css`color: ${({theme}) => theme.text.primary}` + // When __withStyles calls getDynamicPatch({}), theme is {} so theme.text is undefined + const getDynamicPatch = (props: any) => ({ + color: props.theme.text.primary, // This throws: Cannot read properties of undefined + }); + + const metadata: any = { + getDynamicPatch, + }; + + // Should not throw - gracefully handles the error + expect(() => css.__withStyles(metadata)).not.toThrow(); + + // Should return empty styles (no dynamic patch applied) + const result = css.__withStyles(metadata); + expect(result).toEqual([]); + }); + + test('should not crash when JSON.stringify encounters circular references', () => { + const circular: any = { color: 'red' }; + circular.self = circular; + + const getDynamicPatch = () => circular; + + const metadata: any = { + getDynamicPatch, + }; + + // Should not throw + expect(() => css.__withStyles(metadata)).not.toThrow(); + + // Should still push the patch even without caching + const result = css.__withStyles(metadata); + expect(result).toBeDefined(); + }); }); describe('Hybrid Styles (Static + Dynamic)', () => { diff --git a/packages/kstyled/src/css.tsx b/packages/kstyled/src/css.tsx index 61acba7..02c0312 100644 --- a/packages/kstyled/src/css.tsx +++ b/packages/kstyled/src/css.tsx @@ -205,25 +205,37 @@ export const css: CssFactory = Object.assign( // The dynamic values are captured in the closure when Babel plugin creates this function // So SCREEN_WIDTH and other module-level constants work correctly if (getDynamicPatch) { - const dynamicPatch = getDynamicPatch({}); + try { + const dynamicPatch = getDynamicPatch({}); - if (dynamicPatch && Object.keys(dynamicPatch).length > 0) { - // Automatic memoization: Create hash from dynamic values - const hash = JSON.stringify(dynamicPatch, (_key, value) => - value === undefined ? '__ks__undefined__' : value - ); + if (dynamicPatch && Object.keys(dynamicPatch).length > 0) { + // Automatic memoization: Create hash from dynamic values + let hash: string; + try { + hash = JSON.stringify(dynamicPatch, (_key, value) => + value === undefined ? '__ks__undefined__' : value + ); + } catch { + // Fallback for non-serializable values (e.g., circular references on web) + hash = ''; + } - // Check if we can reuse the cached patch - if (metadata._cachedDynamic && metadata._cachedDynamic.hash === hash) { - styles.push(metadata._cachedDynamic.patch); - } else { - // Cache miss: store new patch - metadata._cachedDynamic = { - patch: dynamicPatch, - hash: hash, - }; - styles.push(dynamicPatch); + if (hash && metadata._cachedDynamic && metadata._cachedDynamic.hash === hash) { + // Check if we can reuse the cached patch + styles.push(metadata._cachedDynamic.patch); + } else { + // Cache miss or non-hashable: store new patch + if (hash) { + metadata._cachedDynamic = { patch: dynamicPatch, hash }; + } + styles.push(dynamicPatch); + } } + } catch { + // getDynamicPatch({}) may fail when it contains theme-dependent + // functions (e.g., ({theme}) => theme.bg.basic) since {} has no theme. + // This is expected for css`` used inside styled components where + // theme is provided at render time, not at definition time. } } From 54d2d969c26df1ecd17f53d2be7778f09c5784a3 Mon Sep 17 00:00:00 2001 From: hyochan Date: Fri, 13 Mar 2026 16:22:20 +0900 Subject: [PATCH 2/3] chore: add Claude Code configuration and commands Co-Authored-By: Claude Opus 4.6 --- .claude/commands/commit.md | 167 ++++++++++++++++++++++++++++++++++ .claude/commands/review-pr.md | 111 ++++++++++++++++++++++ 2 files changed, 278 insertions(+) create mode 100644 .claude/commands/commit.md create mode 100644 .claude/commands/review-pr.md diff --git a/.claude/commands/commit.md b/.claude/commands/commit.md new file mode 100644 index 0000000..ad8b21e --- /dev/null +++ b/.claude/commands/commit.md @@ -0,0 +1,167 @@ +# Commit Changes + +Complete workflow: branch → commit → push → PR + +## Usage + +``` +/commit [options] +``` + +**Options:** + +- `--push` or `-p`: Push to remote after commit +- `--pr`: Create PR after push +- `--all` or `-a`: Commit all changes at once +- ``: Commit only specific path (e.g., `packages/kstyled`, `packages/babel-plugin-kstyled`) + +## Examples + +```bash +# Full workflow: commit src changes, push, create PR +/commit packages/kstyled --pr + +# Commit all and create PR +/commit --all --pr + +# Just commit specific path +/commit packages/babel-plugin-kstyled +``` + +## Complete Workflow + +### 1. Check Branch + +```bash +# Check current branch +git branch --show-current +``` + +**If on `main`** → Create a feature branch first: + +```bash +git checkout -b feat/ +``` + +**If NOT on `main`** → Proceed with commits directly. + +**Branch naming conventions:** + +- `feat/` - New features +- `fix/` - Bug fixes +- `docs/` - Documentation only +- `chore/` - Maintenance tasks + +### 2. Pre-Commit Checks (CRITICAL) + +Before staging any changes, run the following checks: + +```bash +# Lint check +bun run lint + +# Type check +bun run typecheck + +# Run tests +bun run test +``` + +**IMPORTANT:** Only proceed with commit if ALL checks pass. + +### 3. Check Current Status + +```bash +git status +git diff --name-only +``` + +### 4. Stage Changes + +**kstyled package:** + +```bash +git add packages/kstyled/ +``` + +**Babel plugin:** + +```bash +git add packages/babel-plugin-kstyled/ +``` + +**All changes:** + +```bash +git add . +``` + +### 5. Review Staged Changes + +```bash +git diff --cached --stat +git diff --cached --name-only +``` + +### 6. Create Commit + +Follow Angular Conventional Commit format: + +```bash +git commit -m "$(cat <<'EOF' +(): + + + +Co-Authored-By: Claude Opus 4.6 +EOF +)" +``` + +**Commit Types:** | Type | Description | |------|-------------| | `feat` | New feature | | `fix` | Bug fix | | `docs` | Documentation only | | `refactor` | Code refactoring | | `chore` | Maintenance tasks | | `test` | Adding/updating tests | | `perf` | Performance improvement | | `style` | Code style (formatting, semicolons, etc.) | + +**Scope Examples:** + +- `css` - css`` tagged template helper +- `styled` - styled component system +- `theme` - Theme provider +- `babel` - Babel plugin changes +- `types` - TypeScript type definitions + +### 7. Push to Remote + +```bash +git push -u origin +``` + +### 8. Create Pull Request + +```bash +gh pr create --title "(): " --body "$(cat <<'EOF' +## Summary + +<1-3 bullet points describing changes> + +## Changes + +- Change 1 +- Change 2 + +## Test plan + +- [ ] `bun run lint` passes +- [ ] `bun run typecheck` passes +- [ ] `bun run test` passes + +🤖 Generated with [Claude Code](https://claude.ai/code) +EOF +)" +``` + +--- + +## Important Notes + +- **ALWAYS** run pre-commit checks before committing +- **ALWAYS** include `Co-Authored-By` footer for Claude-assisted commits +- Use `bun` exclusively for all package management diff --git a/.claude/commands/review-pr.md b/.claude/commands/review-pr.md new file mode 100644 index 0000000..a2478c0 --- /dev/null +++ b/.claude/commands/review-pr.md @@ -0,0 +1,111 @@ +# Review PR Comments + +Review and address PR review comments for this repository. + +## Arguments + +- `$ARGUMENTS` - PR number (e.g., `123`) or PR URL + +## Project-Specific Build Commands + +Based on changed files, run these checks BEFORE committing: + +| Path | Commands | +| --- | --- | +| `packages/kstyled/` | `bun run lint && bun run typecheck && bun run test` | +| `packages/babel-plugin-kstyled/` | `bun run lint && bun run typecheck && bun run test` | + +## Workflow + +### Step 1: Get PR Information + +```bash +# Get PR review comments +gh api repos/{owner}/{repo}/pulls/{pr_number}/comments + +# Get PR reviews (approve, request changes, etc.) +gh api repos/{owner}/{repo}/pulls/{pr_number}/reviews + +# Get changed files +gh pr view {pr_number} --json files +``` + +### Step 2: Analyze Each Comment + +For each review comment: + +1. `path` - File path +2. `line` or `original_line` - Line number +3. `body` - Review content +4. `diff_hunk` - Code context +5. Determine if code change is needed + +### Step 3: Apply Fixes + +1. Read the target file +2. Apply changes per reviewer feedback +3. Track changes with TodoWrite +4. Run project-specific checks + +### Step 4: Run Checks Before Commit + +```bash +bun run lint +bun run typecheck +bun run test +``` + +### Step 5: Commit Changes + +```bash +git add +git commit -m "$(cat <<'EOF' +fix: address PR review comments + +- +- + +Co-Authored-By: Claude Opus 4.6 +EOF +)" +``` + +### Step 6: Reply to Comments + +Reply to each addressed comment: + +```bash +gh api repos/{owner}/{repo}/pulls/{pr_number}/comments/{comment_id}/replies \ + -X POST -f body="Fixed in abc1234. + +**Changes:** +- Description of what was changed" +``` + +## Reply Format Rules (CRITICAL) + +When replying to PR comments: + +### Commit Hash Formatting + +**NEVER wrap commit hashes in backticks or code blocks.** GitHub only auto-links plain text commit hashes. + +| Format | Example | Result | +| ---------- | ----------------------- | ------------------------ | +| CORRECT | `Fixed in f3b5fec.` | Clickable link to commit | +| WRONG | `Fixed in \`f3b5fec\`.` | Plain text, no link | + +## Result Report + +After addressing all comments, report: + +- List of modified files +- Summary of changes per file +- Commit hash +- Any comments not addressed and why + +## Notes + +- If a comment is a question or praise, no code change needed +- If reviewer intent is unclear, ask for clarification +- Use `bun` exclusively for all commands From 1c7f84243e1c6d20e28aa13c0e51efc2af92f3f0 Mon Sep 17 00:00:00 2001 From: hyochan Date: Fri, 13 Mar 2026 16:48:20 +0900 Subject: [PATCH 3/3] fix: address PR review comments - Strengthen circular reference test assertion (toBe instead of toBeDefined) - Assert _cachedDynamic is undefined when serialization fails - Add __DEV__ debug logging for swallowed getDynamicPatch errors - Fix markdown lint: add language tag to fenced code block - Fix contradictory commit-hash formatting example Co-Authored-By: Claude Opus 4.6 --- .claude/commands/commit.md | 2 +- .claude/commands/review-pr.md | 2 +- packages/kstyled/src/__tests__/runtime.test.tsx | 3 ++- packages/kstyled/src/css.tsx | 5 ++++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/.claude/commands/commit.md b/.claude/commands/commit.md index ad8b21e..c42f270 100644 --- a/.claude/commands/commit.md +++ b/.claude/commands/commit.md @@ -4,7 +4,7 @@ Complete workflow: branch → commit → push → PR ## Usage -``` +```bash /commit [options] ``` diff --git a/.claude/commands/review-pr.md b/.claude/commands/review-pr.md index a2478c0..3c8e0a1 100644 --- a/.claude/commands/review-pr.md +++ b/.claude/commands/review-pr.md @@ -92,7 +92,7 @@ When replying to PR comments: | Format | Example | Result | | ---------- | ----------------------- | ------------------------ | -| CORRECT | `Fixed in f3b5fec.` | Clickable link to commit | +| CORRECT | Fixed in f3b5fec. | Clickable link to commit | | WRONG | `Fixed in \`f3b5fec\`.` | Plain text, no link | ## Result Report diff --git a/packages/kstyled/src/__tests__/runtime.test.tsx b/packages/kstyled/src/__tests__/runtime.test.tsx index e15e705..509a406 100644 --- a/packages/kstyled/src/__tests__/runtime.test.tsx +++ b/packages/kstyled/src/__tests__/runtime.test.tsx @@ -461,7 +461,8 @@ describe('kstyled Runtime Tests', () => { // Should still push the patch even without caching const result = css.__withStyles(metadata); - expect(result).toBeDefined(); + expect(result).toBe(circular); + expect(metadata._cachedDynamic).toBeUndefined(); }); }); diff --git a/packages/kstyled/src/css.tsx b/packages/kstyled/src/css.tsx index 02c0312..9677718 100644 --- a/packages/kstyled/src/css.tsx +++ b/packages/kstyled/src/css.tsx @@ -231,11 +231,14 @@ export const css: CssFactory = Object.assign( styles.push(dynamicPatch); } } - } catch { + } catch (error) { // getDynamicPatch({}) may fail when it contains theme-dependent // functions (e.g., ({theme}) => theme.bg.basic) since {} has no theme. // This is expected for css`` used inside styled components where // theme is provided at render time, not at definition time. + if (typeof __DEV__ !== 'undefined' && __DEV__) { + console.warn('[kstyled] css.__withStyles getDynamicPatch({}) failed:', error); + } } }