Skip to content

Commit a2b2233

Browse files
hyochanclaude
hyochan
and
claude
authored
fix: handle errors in css.__withStyles getDynamicPatch (#5)
## Summary - Add try-catch around `getDynamicPatch({})` in `css.__withStyles` to prevent crashes when theme-dependent functions are called with empty props - Add try-catch around `JSON.stringify(dynamicPatch)` to handle circular references (e.g., DOM elements on web) - Skip caching for non-serializable patches instead of crashing ## Test plan - [x] Added test: `should not crash when getDynamicPatch throws (theme-dependent css``)` - [x] Added test: `should not crash when JSON.stringify encounters circular references` - [ ] Verify web rendering works without circular reference errors 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Dynamic CSS patching now handles errors and non-serializable values without crashing. * **Tests** * Added tests covering serialization edge cases and error scenarios for dynamic CSS patches. * **Documentation** * Added developer workflow guides for committing changes and reviewing PRs, including commit/PR templates and step-by-step checklists. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 9a54fe0 commit a2b2233

4 files changed

Lines changed: 347 additions & 16 deletions

File tree

‎.claude/commands/commit.md‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
# Commit Changes
2+
3+
Complete workflow: branch → commit → push → PR
4+
5+
## Usage
6+
7+
```bash
8+
/commit [options]
9+
```
10+
11+
**Options:**
12+
13+
- `--push` or `-p`: Push to remote after commit
14+
- `--pr`: Create PR after push
15+
- `--all` or `-a`: Commit all changes at once
16+
- `<path>`: Commit only specific path (e.g., `packages/kstyled`, `packages/babel-plugin-kstyled`)
17+
18+
## Examples
19+
20+
```bash
21+
# Full workflow: commit src changes, push, create PR
22+
/commit packages/kstyled --pr
23+
24+
# Commit all and create PR
25+
/commit --all --pr
26+
27+
# Just commit specific path
28+
/commit packages/babel-plugin-kstyled
29+
```
30+
31+
## Complete Workflow
32+
33+
### 1. Check Branch
34+
35+
```bash
36+
# Check current branch
37+
git branch --show-current
38+
```
39+
40+
**If on `main`** → Create a feature branch first:
41+
42+
```bash
43+
git checkout -b feat/<feature-name>
44+
```
45+
46+
**If NOT on `main`** → Proceed with commits directly.
47+
48+
**Branch naming conventions:**
49+
50+
- `feat/<feature-name>` - New features
51+
- `fix/<bug-description>` - Bug fixes
52+
- `docs/<doc-update>` - Documentation only
53+
- `chore/<task>` - Maintenance tasks
54+
55+
### 2. Pre-Commit Checks (CRITICAL)
56+
57+
Before staging any changes, run the following checks:
58+
59+
```bash
60+
# Lint check
61+
bun run lint
62+
63+
# Type check
64+
bun run typecheck
65+
66+
# Run tests
67+
bun run test
68+
```
69+
70+
**IMPORTANT:** Only proceed with commit if ALL checks pass.
71+
72+
### 3. Check Current Status
73+
74+
```bash
75+
git status
76+
git diff --name-only
77+
```
78+
79+
### 4. Stage Changes
80+
81+
**kstyled package:**
82+
83+
```bash
84+
git add packages/kstyled/
85+
```
86+
87+
**Babel plugin:**
88+
89+
```bash
90+
git add packages/babel-plugin-kstyled/
91+
```
92+
93+
**All changes:**
94+
95+
```bash
96+
git add .
97+
```
98+
99+
### 5. Review Staged Changes
100+
101+
```bash
102+
git diff --cached --stat
103+
git diff --cached --name-only
104+
```
105+
106+
### 6. Create Commit
107+
108+
Follow Angular Conventional Commit format:
109+
110+
```bash
111+
git commit -m "$(cat <<'EOF'
112+
<type>(<scope>): <description>
113+
114+
<body - what changed and why>
115+
116+
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
117+
EOF
118+
)"
119+
```
120+
121+
**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.) |
122+
123+
**Scope Examples:**
124+
125+
- `css` - css`` tagged template helper
126+
- `styled` - styled component system
127+
- `theme` - Theme provider
128+
- `babel` - Babel plugin changes
129+
- `types` - TypeScript type definitions
130+
131+
### 7. Push to Remote
132+
133+
```bash
134+
git push -u origin <branch-name>
135+
```
136+
137+
### 8. Create Pull Request
138+
139+
```bash
140+
gh pr create --title "<type>(<scope>): <description>" --body "$(cat <<'EOF'
141+
## Summary
142+
143+
<1-3 bullet points describing changes>
144+
145+
## Changes
146+
147+
- Change 1
148+
- Change 2
149+
150+
## Test plan
151+
152+
- [ ] `bun run lint` passes
153+
- [ ] `bun run typecheck` passes
154+
- [ ] `bun run test` passes
155+
156+
🤖 Generated with [Claude Code](https://claude.ai/code)
157+
EOF
158+
)"
159+
```
160+
161+
---
162+
163+
## Important Notes
164+
165+
- **ALWAYS** run pre-commit checks before committing
166+
- **ALWAYS** include `Co-Authored-By` footer for Claude-assisted commits
167+
- Use `bun` exclusively for all package management

‎.claude/commands/review-pr.md‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
# Review PR Comments
2+
3+
Review and address PR review comments for this repository.
4+
5+
## Arguments
6+
7+
- `$ARGUMENTS` - PR number (e.g., `123`) or PR URL
8+
9+
## Project-Specific Build Commands
10+
11+
Based on changed files, run these checks BEFORE committing:
12+
13+
| Path | Commands |
14+
| --- | --- |
15+
| `packages/kstyled/` | `bun run lint && bun run typecheck && bun run test` |
16+
| `packages/babel-plugin-kstyled/` | `bun run lint && bun run typecheck && bun run test` |
17+
18+
## Workflow
19+
20+
### Step 1: Get PR Information
21+
22+
```bash
23+
# Get PR review comments
24+
gh api repos/{owner}/{repo}/pulls/{pr_number}/comments
25+
26+
# Get PR reviews (approve, request changes, etc.)
27+
gh api repos/{owner}/{repo}/pulls/{pr_number}/reviews
28+
29+
# Get changed files
30+
gh pr view {pr_number} --json files
31+
```
32+
33+
### Step 2: Analyze Each Comment
34+
35+
For each review comment:
36+
37+
1. `path` - File path
38+
2. `line` or `original_line` - Line number
39+
3. `body` - Review content
40+
4. `diff_hunk` - Code context
41+
5. Determine if code change is needed
42+
43+
### Step 3: Apply Fixes
44+
45+
1. Read the target file
46+
2. Apply changes per reviewer feedback
47+
3. Track changes with TodoWrite
48+
4. Run project-specific checks
49+
50+
### Step 4: Run Checks Before Commit
51+
52+
```bash
53+
bun run lint
54+
bun run typecheck
55+
bun run test
56+
```
57+
58+
### Step 5: Commit Changes
59+
60+
```bash
61+
git add <changed-files>
62+
git commit -m "$(cat <<'EOF'
63+
fix: address PR review comments
64+
65+
- <summary of change 1>
66+
- <summary of change 2>
67+
68+
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
69+
EOF
70+
)"
71+
```
72+
73+
### Step 6: Reply to Comments
74+
75+
Reply to each addressed comment:
76+
77+
```bash
78+
gh api repos/{owner}/{repo}/pulls/{pr_number}/comments/{comment_id}/replies \
79+
-X POST -f body="Fixed in abc1234.
80+
81+
**Changes:**
82+
- Description of what was changed"
83+
```
84+
85+
## Reply Format Rules (CRITICAL)
86+
87+
When replying to PR comments:
88+
89+
### Commit Hash Formatting
90+
91+
**NEVER wrap commit hashes in backticks or code blocks.** GitHub only auto-links plain text commit hashes.
92+
93+
| Format | Example | Result |
94+
| ---------- | ----------------------- | ------------------------ |
95+
| CORRECT | Fixed in f3b5fec. | Clickable link to commit |
96+
| WRONG | `Fixed in \`f3b5fec\`.` | Plain text, no link |
97+
98+
## Result Report
99+
100+
After addressing all comments, report:
101+
102+
- List of modified files
103+
- Summary of changes per file
104+
- Commit hash
105+
- Any comments not addressed and why
106+
107+
## Notes
108+
109+
- If a comment is a question or praise, no code change needed
110+
- If reviewer intent is unclear, ask for clarification
111+
- Use `bun` exclusively for all commands

‎packages/kstyled/src/__tests__/runtime.test.tsx‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,44 @@ describe('kstyled Runtime Tests', () => {
426426
expect(patch.marginTop).toBe(32);
427427
expect(patch.backgroundColor).toBe('#007AFF');
428428
});
429+
430+
test('should not crash when getDynamicPatch throws (theme-dependent css``)', () => {
431+
// Simulates compiled css`` with theme interpolation: css`color: ${({theme}) => theme.text.primary}`
432+
// When __withStyles calls getDynamicPatch({}), theme is {} so theme.text is undefined
433+
const getDynamicPatch = (props: any) => ({
434+
color: props.theme.text.primary, // This throws: Cannot read properties of undefined
435+
});
436+
437+
const metadata: any = {
438+
getDynamicPatch,
439+
};
440+
441+
// Should not throw - gracefully handles the error
442+
expect(() => css.__withStyles(metadata)).not.toThrow();
443+
444+
// Should return empty styles (no dynamic patch applied)
445+
const result = css.__withStyles(metadata);
446+
expect(result).toEqual([]);
447+
});
448+
449+
test('should not crash when JSON.stringify encounters circular references', () => {
450+
const circular: any = { color: 'red' };
451+
circular.self = circular;
452+
453+
const getDynamicPatch = () => circular;
454+
455+
const metadata: any = {
456+
getDynamicPatch,
457+
};
458+
459+
// Should not throw
460+
expect(() => css.__withStyles(metadata)).not.toThrow();
461+
462+
// Should still push the patch even without caching
463+
const result = css.__withStyles(metadata);
464+
expect(result).toBe(circular);
465+
expect(metadata._cachedDynamic).toBeUndefined();
466+
});
429467
});
430468

431469
describe('Hybrid Styles (Static + Dynamic)', () => {

‎packages/kstyled/src/css.tsx‎

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -205,24 +205,39 @@ export const css: CssFactory = Object.assign(
205205
// The dynamic values are captured in the closure when Babel plugin creates this function
206206
// So SCREEN_WIDTH and other module-level constants work correctly
207207
if (getDynamicPatch) {
208-
const dynamicPatch = getDynamicPatch({});
208+
try {
209+
const dynamicPatch = getDynamicPatch({});
209210

210-
if (dynamicPatch && Object.keys(dynamicPatch).length > 0) {
211-
// Automatic memoization: Create hash from dynamic values
212-
const hash = JSON.stringify(dynamicPatch, (_key, value) =>
213-
value === undefined ? '__ks__undefined__' : value
214-
);
211+
if (dynamicPatch && Object.keys(dynamicPatch).length > 0) {
212+
// Automatic memoization: Create hash from dynamic values
213+
let hash: string;
214+
try {
215+
hash = JSON.stringify(dynamicPatch, (_key, value) =>
216+
value === undefined ? '__ks__undefined__' : value
217+
);
218+
} catch {
219+
// Fallback for non-serializable values (e.g., circular references on web)
220+
hash = '';
221+
}
215222

216-
// Check if we can reuse the cached patch
217-
if (metadata._cachedDynamic && metadata._cachedDynamic.hash === hash) {
218-
styles.push(metadata._cachedDynamic.patch);
219-
} else {
220-
// Cache miss: store new patch
221-
metadata._cachedDynamic = {
222-
patch: dynamicPatch,
223-
hash: hash,
224-
};
225-
styles.push(dynamicPatch);
223+
if (hash && metadata._cachedDynamic && metadata._cachedDynamic.hash === hash) {
224+
// Check if we can reuse the cached patch
225+
styles.push(metadata._cachedDynamic.patch);
226+
} else {
227+
// Cache miss or non-hashable: store new patch
228+
if (hash) {
229+
metadata._cachedDynamic = { patch: dynamicPatch, hash };
230+
}
231+
styles.push(dynamicPatch);
232+
}
233+
}
234+
} catch (error) {
235+
// getDynamicPatch({}) may fail when it contains theme-dependent
236+
// functions (e.g., ({theme}) => theme.bg.basic) since {} has no theme.
237+
// This is expected for css`` used inside styled components where
238+
// theme is provided at render time, not at definition time.
239+
if (typeof __DEV__ !== 'undefined' && __DEV__) {
240+
console.warn('[kstyled] css.__withStyles getDynamicPatch({}) failed:', error);
226241
}
227242
}
228243
}

0 commit comments

Comments
 (0)