fix: comprehensive audit — core correctness, 30-100x smaller PNGs, react stability, packaging & CI fixes#6
Merged
Merged
Conversation
…ity, packaging, CI Core (@react-pixel-ui/core): - PNG encoder: real DEFLATE (fixed Huffman + LZ77), 30-100x smaller data URLs - generatePixelArt: options.want to skip unused PNG output + LRU result cache - Colors: full 148 CSS named colors, color(srgb/srgb-linear/display-p3), anchored rgb()/rgba() patterns - Gradients: 'in <colorspace>' interpolation hints (Tailwind v4), turn/rad/grad units, aspect-ratio magic corners, double-position stops, px-position stops, -1 sentinel fix - pixel-shadow: zero offsets stay zero; nearest-grid rounding - composite image: transparent interior for border-only elements; null for unparseable backgrounds (graceful degradation) - parseComputedStyles: % border-radius resolution, elliptical radii - parseBoxShadow: detect trailing 'inset' (computed-style order), shadow lists - sanitizePixelSize guards in every generator (no more hangs on pixelSize 0) React (@react-pixel-ui/react): - Pixel: sticky shadow wrapper (no child DOM remount), ResizeObserver re-measure + zero-size mount retry, graceful degradation - usePixelRef: observer lifecycle owned by effect (StrictMode-safe), borderless shadow fix, graceful degradation - PixelBox: responsive mode unpinned (actually follows container), style/className land on root, displayName - PixelButton: unknown-variant fallback, type=button default, displayName - PixelConfigProvider: optional config prop - usePixelArt: per-corner radius memo by value; skips composite output Packaging & repo: - exports: per-condition types (.d.mts for ESM) — fixes attw FalseCJS - react tsup: sourcemap-aware 'use client' banner (maps no longer off by one) - release.yml: remove placeholder my-slack-bot step that failed every publish run; frozen lockfile; release script now runs tests - actions: checkout/setup-node/cache bumped to v5 (node24 runtime) - eslint-config/typescript-config: private (no more accidental npm publish) - changesets: changelog-github with repo links - remove stale scripts/update-configs.js, debug-test.js; turbo outputs fixed - root CHANGELOG → stub + docs/MIGRATION.md Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 39 minutes and 18 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Walkthrough이 PR은 react-pixel-ui v2 릴리스를 위한 중대한 개선사항들을 담고 있습니다. Changesets 기반 자동 버저닝 설정, PNG DEFLATE 압축, CSS Color 4 기반 색상/그라디언트 파싱 강화, React 컴포넌트의 ResizeObserver 기반 라이프사이클 안정화, 그리고 responsive 크기 조정 개선이 핵심입니다. ChangesRelease Infrastructure & Configuration
Core Package - Utilities & Fundamentals
Core Package - Generators & Composition
Core Package - Public API
React Components
React Hooks
Documentation
🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
|
… corepack after) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/react/src/components/Pixel.tsx (1)
117-148: ⚡ Quick winResizeObserver가
artState변경 시마다 재생성됩니다.
artState가 의존성 배열에 포함되어 있어, 크기가 변경될 때마다 observer가 disconnect/reconnect됩니다. 콜백 내부에서artState를 ref로 접근하면 observer 재생성 없이 최신 값을 참조할 수 있습니다.♻️ ref를 사용한 개선안
+ const artStateRef = useRef<ArtState | null>(null); + artStateRef.current = artState; + useEffect(() => { if (!enabled) return; if (typeof ResizeObserver === 'undefined') return; const el = childRef.current; if (!el) return; let rafId: number | null = null; const observer = new ResizeObserver(() => { if (rafId !== null) return; rafId = requestAnimationFrame(() => { rafId = null; const node = childRef.current; if (!node) return; const w = node.offsetWidth; const h = node.offsetHeight; - if (artState) { - if (w !== artState.width || h !== artState.height) { + const currentArt = artStateRef.current; + if (currentArt) { + if (w !== currentArt.width || h !== currentArt.height) { setArtState(null); setMeasureToken((t) => t + 1); } } else if (w > 0 && h > 0) { setMeasureToken((t) => t + 1); } }); }); observer.observe(el); return () => { if (rafId !== null) cancelAnimationFrame(rafId); observer.disconnect(); }; - }, [enabled, artState]); + }, [enabled]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react/src/components/Pixel.tsx` around lines 117 - 148, The effect recreates the ResizeObserver on every artState change because artState is in the dependency array; instead keep the observer stable by removing artState from the deps and reading the latest value via a ref: create an artStateRef (synchronized whenever artState changes) and use artStateRef.current inside the ResizeObserver callback to compare widths/heights and decide whether to setArtState(null) or setMeasureToken(...); keep other logic and cleanup (cancelAnimationFrame, observer.disconnect) the same and ensure artStateRef is updated whenever artState changes so the observer always sees the latest value without being recreated.packages/core/src/composer.ts (1)
79-79: 💤 Low valueJSON.stringify 키 생성 시 프로퍼티 순서에 따른 캐시 미스 가능성
JSON.stringify(config)는 객체 프로퍼티 생성 순서에 민감할 수 있습니다. 예를 들어{borderColor: 'red', borderWidth: 1}과{borderWidth: 1, borderColor: 'red'}는 동일한 설정이지만 다른 캐시 키를 생성할 수 있습니다.현재 V8 엔진은 삽입 순서를 보존하므로 실무에서는 문제가 적지만, 캐시 적중률이 예상보다 낮다면 정규화된 키 생성(예: 정렬된 키 기반)을 고려할 수 있습니다. LRU 캐시이므로 오탐(false positive)이 아닌 누락(false negative)만 발생해 정확성에는 영향이 없습니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/composer.ts` at line 79, The cache key uses JSON.stringify(config) which is sensitive to object property order and can cause cache misses; replace that with a deterministic serialization of config (e.g., implement or use a stable stringify that sorts object keys recursively) and use it in the cacheKey expression (keep the rest: `${width}|${height}|${want}|${...}`), ensuring the unique symbol names cacheKey, width, height, want, and config are preserved while normalizing config before concatenation.packages/core/src/utils/png.ts (1)
222-259: 💤 Low valueLZ77 매칭 루프의 암묵적 경계 검사
Line 233의
data[j + l] === data[i + l]비교에서j + l >= n인 경우data[j + l]은undefined를 반환하며, 이는data[i + l](유효한 숫자)와 비교 시false가 되어 루프가 종료됩니다.이는 JavaScript의 Uint8Array 범위 외 접근 동작에 의존하는 암묵적 안전 장치입니다. 테스트가 통과하고(대형 이미지 포함) 동작은 정확하지만, 명시적인 경계 검사(예:
j + l < n)를 추가하면 의도가 더 명확해집니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/utils/png.ts` around lines 222 - 259, The inner match-length loop in the LZ77 matcher (inside the while over i) relies on out-of-bounds reads returning undefined; update the loop condition used when computing l (currently inside the block starting with const h = hashAt(i) and the inner while with data[j + l] === data[i + l]) to explicitly check bounds by adding a j + l < n check (so it becomes while (l < maxLen && j + l < n && data[j + l] === data[i + l]) ...). This keeps the existing maxLen logic (Math.min(258, n - i)) for i-side safety and only makes the j-side boundary explicit; no other logic changes are required (keep prev[i] = head[h], head[h] = i and subsequent match handling).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/MIGRATION.md`:
- Line 3: The migration note currently hard-codes the release date string
"v2.0.0 (2026-04-03)" in MIGRATION.md; update this to either the final release
date when known or replace the literal date with a placeholder token (e.g.,
"v2.0.0 (RELEASE_DATE)") so the document won't become stale while the PR is
open, and ensure any CI/docs generation that consumes this file can substitute
the real date later.
In `@packages/core/src/computed-style-parser.ts`:
- Around line 19-39: In resolveRadius, the current logic returns the raw
percentage number when size is missing which yields a unitless value; change the
percentage fallback so that when component.endsWith('%') and size is undefined
you return 0 (not n), ensuring percentage radii without size resolve to 0px;
update the function comment/JSDoc to note that percentage values require size
and will fallback to 0 when absent (function: resolveRadius, local symbols:
components, resolved).
In `@packages/core/src/utils/math.ts`:
- Around line 8-10: sanitizePixelSize currently lets Infinity (and non-finite
values) through because pixelSize || 4 doesn't guard against Infinity; update
sanitizePixelSize to treat only finite numeric pixelSize as valid and otherwise
fall back to 4, then floor and clamp to a minimum of 1. Specifically, in
function sanitizePixelSize, check Number.isFinite(pixelSize) (and that it's > 0)
before using it; otherwise use 4, then apply Math.floor and Math.max as before
to return a safe integer.
---
Nitpick comments:
In `@packages/core/src/composer.ts`:
- Line 79: The cache key uses JSON.stringify(config) which is sensitive to
object property order and can cause cache misses; replace that with a
deterministic serialization of config (e.g., implement or use a stable stringify
that sorts object keys recursively) and use it in the cacheKey expression (keep
the rest: `${width}|${height}|${want}|${...}`), ensuring the unique symbol names
cacheKey, width, height, want, and config are preserved while normalizing config
before concatenation.
In `@packages/core/src/utils/png.ts`:
- Around line 222-259: The inner match-length loop in the LZ77 matcher (inside
the while over i) relies on out-of-bounds reads returning undefined; update the
loop condition used when computing l (currently inside the block starting with
const h = hashAt(i) and the inner while with data[j + l] === data[i + l]) to
explicitly check bounds by adding a j + l < n check (so it becomes while (l <
maxLen && j + l < n && data[j + l] === data[i + l]) ...). This keeps the
existing maxLen logic (Math.min(258, n - i)) for i-side safety and only makes
the j-side boundary explicit; no other logic changes are required (keep prev[i]
= head[h], head[h] = i and subsequent match handling).
In `@packages/react/src/components/Pixel.tsx`:
- Around line 117-148: The effect recreates the ResizeObserver on every artState
change because artState is in the dependency array; instead keep the observer
stable by removing artState from the deps and reading the latest value via a
ref: create an artStateRef (synchronized whenever artState changes) and use
artStateRef.current inside the ResizeObserver callback to compare widths/heights
and decide whether to setArtState(null) or setMeasureToken(...); keep other
logic and cleanup (cancelAnimationFrame, observer.disconnect) the same and
ensure artStateRef is updated whenever artState changes so the observer always
sees the latest value without being recreated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 80a70e7d-f6ea-42ef-ac44-d3dab7fceadc
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (48)
.changeset/config.json.changeset/core-audit-improvements.md.changeset/react-audit-improvements.md.github/workflows/ci.yml.github/workflows/release.ymlCHANGELOG.mdREADME.mddebug-test.jsdocs/MIGRATION.mdpackage.jsonpackages/core/package.jsonpackages/core/src/composer.test.tspackages/core/src/composer.tspackages/core/src/computed-style-parser.tspackages/core/src/generators/composite-pixel-image.test.tspackages/core/src/generators/composite-pixel-image.tspackages/core/src/generators/pixel-gradient.tspackages/core/src/generators/pixel-shadow.test.tspackages/core/src/generators/pixel-shadow.tspackages/core/src/generators/staircase-polygon.tspackages/core/src/generators/stepped-gradient.tspackages/core/src/index.tspackages/core/src/types.tspackages/core/src/utils/box-shadow-parser.tspackages/core/src/utils/color.test.tspackages/core/src/utils/color.tspackages/core/src/utils/gradient-parser.test.tspackages/core/src/utils/gradient-parser.tspackages/core/src/utils/math.tspackages/core/src/utils/png.test.tspackages/core/src/utils/png.tspackages/eslint-config/package.jsonpackages/react/package.jsonpackages/react/src/components/Pixel.test.tsxpackages/react/src/components/Pixel.tsxpackages/react/src/components/PixelBox.test.tsxpackages/react/src/components/PixelBox.tsxpackages/react/src/components/PixelButton.test.tsxpackages/react/src/components/PixelButton.tsxpackages/react/src/context/PixelConfigProvider.test.tsxpackages/react/src/context/PixelConfigProvider.tsxpackages/react/src/hooks/usePixelArt.tspackages/react/src/hooks/usePixelRef.tspackages/react/src/hooks/useResponsiveSize.tspackages/react/tsup.config.tspackages/typescript-config/package.jsonscripts/update-configs.jsturbo.json
💤 Files with no reviewable changes (2)
- debug-test.js
- scripts/update-configs.js
| @@ -0,0 +1,71 @@ | |||
| # v1 → v2 Migration Guide | |||
|
|
|||
| v2.0.0 (2026-04-03) was a full rewrite. This guide covers what changed | |||
There was a problem hiding this comment.
릴리스 날짜를 확인하세요.
문서에 2026-04-03이 v2.0.0 릴리스 날짜로 명시되어 있습니다. 현재 6월이고 PR이 아직 open 상태이므로, 실제 릴리스 시 날짜를 업데이트해야 할 수 있습니다. 또는 플레이스홀더를 사용하는 것도 고려하세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/MIGRATION.md` at line 3, The migration note currently hard-codes the
release date string "v2.0.0 (2026-04-03)" in MIGRATION.md; update this to either
the final release date when known or replace the literal date with a placeholder
token (e.g., "v2.0.0 (RELEASE_DATE)") so the document won't become stale while
the PR is open, and ensure any CI/docs generation that consumes this file can
substitute the real date later.
Comment on lines
+19
to
+39
| function resolveRadius(value: string, size?: ElementSize): number { | ||
| if (!value) return 0; | ||
| const components = value.trim().split(/\s+/).slice(0, 2); | ||
|
|
||
| const resolved = components.map((component, i) => { | ||
| const n = parseFloat(component); | ||
| if (Number.isNaN(n)) return 0; | ||
| if (component.endsWith('%')) { | ||
| if (!size) return n; // no size available — legacy px fallback | ||
| const reference = i === 0 ? size.width : size.height; | ||
| // A single "50%" component applies to both axes — use the smaller | ||
| // reference so the radius fits the element. | ||
| const ref = | ||
| components.length === 1 ? Math.min(size.width, size.height) : reference; | ||
| return (n / 100) * ref; | ||
| } | ||
| return n; | ||
| }); | ||
|
|
||
| return Math.max(0, Math.min(...resolved)); | ||
| } |
There was a problem hiding this comment.
백분율 반지름의 폴백 동작이 잘못되었습니다.
27번 줄에서 size가 없을 때 백분율 값을 그대로 반환합니다(예: "50%" → 50). 이는 픽셀 값이 아니라 단위 없는 숫자이므로 호출자가 픽셀로 해석할 때 부정확한 반지름 계산을 야기합니다.
권장 수정 방안:
- 옵션 1 (가장 안전):
size없이 백분율을 만나면0을 반환 - 옵션 2:
parseComputedStyles의size매개변수를 필수로 변경 - 옵션 3: 현재 동작을 JSDoc에 명확히 문서화하되, 이 경우에도
n대신0을 반환하는 것이 안전합니다.
🔧 제안 수정안 (옵션 1: 해석 불가 시 0 반환)
if (component.endsWith('%')) {
- if (!size) return n; // no size available — legacy px fallback
+ if (!size) return 0; // no size available — can't resolve percentage
const reference = i === 0 ? size.width : size.height;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/computed-style-parser.ts` around lines 19 - 39, In
resolveRadius, the current logic returns the raw percentage number when size is
missing which yields a unitless value; change the percentage fallback so that
when component.endsWith('%') and size is undefined you return 0 (not n),
ensuring percentage radii without size resolve to 0px; update the function
comment/JSDoc to note that percentage values require size and will fallback to 0
when absent (function: resolveRadius, local symbols: components, resolved).
Comment on lines
+8
to
+10
| export function sanitizePixelSize(pixelSize: number): number { | ||
| return Math.max(1, Math.floor(pixelSize || 4)); | ||
| } |
There was a problem hiding this comment.
sanitizePixelSize가 Infinity를 안전값으로 정규화하지 못합니다.
Line 9에서 pixelSize || 4는 Infinity를 그대로 통과시키므로, 하위 연산(예: 그림자 스냅)에서 NaN 결과가 발생할 수 있습니다.
수정 예시
export function sanitizePixelSize(pixelSize: number): number {
- return Math.max(1, Math.floor(pixelSize || 4));
+ const fallback = 4;
+ const normalized = Number.isFinite(pixelSize) ? pixelSize : fallback;
+ return Math.max(1, Math.floor(normalized || fallback));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function sanitizePixelSize(pixelSize: number): number { | |
| return Math.max(1, Math.floor(pixelSize || 4)); | |
| } | |
| export function sanitizePixelSize(pixelSize: number): number { | |
| const fallback = 4; | |
| const normalized = Number.isFinite(pixelSize) ? pixelSize : fallback; | |
| return Math.max(1, Math.floor(normalized || fallback)); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/utils/math.ts` around lines 8 - 10, sanitizePixelSize
currently lets Infinity (and non-finite values) through because pixelSize || 4
doesn't guard against Infinity; update sanitizePixelSize to treat only finite
numeric pixelSize as valid and otherwise fall back to 4, then floor and clamp to
a minimum of 1. Specifically, in function sanitizePixelSize, check
Number.isFinite(pixelSize) (and that it's > 0) before using it; otherwise use 4,
then apply Math.floor and Math.max as before to return a safe integer.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Full audit of both published packages (53-agent multi-dimensional review across packaging, core logic, react layer, performance, repo hygiene, and consumer API behavior), with all 40+ confirmed findings fixed.
@react-pixel-ui/core (minor)
Performance
zlib.inflateSyncround-tripsgeneratePixelArtgainsoptions.want: 'styles' | 'composite' | 'both'— consumers stop generating the PNG they discard (~2× faster), plus a small LRU result cache (hover/theme churn becomes free)Correctness
lime,navy,tomato… failed to parse); newcolor(srgb | srgb-linear | display-p3)support (Chromium's computedcolor-mix()output); anchoredrgb()patterns (no moreconic-gradient(rgb(...))misparsed as solid red)to right in oklabhints,turn/rad/gradunits, aspect-ratio-correct corner keywords (newresolveGradientAngle), double-position stops, px-position stops,-1sentinel corruption fixgeneratePixelShadow: zero offsets stay zero (box-shadow: 0 4pxno longer renders diagonal)nullfor unparseable backgrounds so consumers degrade gracefully instead of erasing the elementparseComputedStyles: percentageborder-radius(50% avatars) resolved against element size; elliptical radiiparseBoxShadow: detects trailinginset(real browsers serialize it last — inset shadows were converted to outer shadows); handles shadow listspixelSize(0 used to hang the tab or throw)@react-pixel-ui/react (minor)
<Pixel>: shadow wrapper no longer remounts the child DOM node on re-measures (input state/focus survive);ResizeObserverre-measures on size changes and zero-size mounts (display:nonetabs now get art when shown); unparseable backgrounds left untouchedusePixelRef: observer lifecycle moved into an effect — React 18 StrictMode no longer permanently disconnects it after mount; borderless elements keep their (pixelated) shadow instead of losing it<PixelBox>:responsiveactually works (no more self-pinned 200×100);style/classNameconsistently land on the root element<PixelButton>: unknown variant falls back to primary (no TypeError);type="button"default; properdisplayNamesPixelConfigProvider:configprop optionalusePixelArt: inlineborderRadiusarrays no longer defeat the memoPackaging / Infra
exportstypes (.d.mtsfor ESM) — attw now reports no problems (was "Masquerading as CJS", which let TS approve code that crashed at runtime under nodenext)"use client"via esbuild banner — source maps no longer off by one linemy-slack-botstep that made every successful npm publish run show as failed; release script now runs tests beforechangeset publish; frozen lockfileprivate(both had leaked to npm before)changelog-github(linked changelogs; validated against this repo)scripts/update-configs.js(wrong paths, destructive),debug-test.js; fixed turbooutputswarnings; root CHANGELOG → stub +docs/MIGRATION.mdTest plan
attwclean on both packed tarballs (node10 / node16-CJS / node16-ESM / bundler)renderToStringchangeset versiondry-run with changelog-github validated🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
generatePixelArt옵션 추가로 불필요한 PNG 생성 스킵 가능color(),color-mix()함수 지원Bug Fixes
PixelBox반응형 모드의 크기 추종 개선PixelButton미지정 variant 값 처리 개선Documentation
Chores