fix: parsing multiple transforms tokens in single class#451
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds detection and flattening of CSS transform arrays in the Metro CSS processor: a TRANSFORM_TYPES set and isTransformArray() helper ensure arrays of transform objects are processed element-wise and normalized to a flat transform array. Includes a test and CSS utility for multiple transforms in one token. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan for PR comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/uniwind/src/metro/processor/css.ts (1)
518-522: Consider adding a null check for defensive coding.
typeof null === 'object'in JavaScript, so ifvaluesever contains anullelement, the'type' in valuecheck would throw a TypeError. While unlikely with lightningcss output, a null guard would be more robust.🛡️ Optional defensive fix
private isTransformArray(values: Array<any>) { return values.every( - value => typeof value === 'object' && 'type' in value && CSS.TRANSFORM_TYPES.has(value.type), + value => typeof value === 'object' && value !== null && 'type' in value && CSS.TRANSFORM_TYPES.has(value.type), ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/uniwind/src/metro/processor/css.ts` around lines 518 - 522, The isTransformArray method can throw if an element is null because typeof null === 'object'; update the predicate in isTransformArray to guard against null/undefined (e.g., check value != null or value !== null) before checking 'type' and CSS.TRANSFORM_TYPES so values.every(...) only accesses properties on non-null objects; keep the rest of the logic (using CSS.TRANSFORM_TYPES.has(value.type)) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/uniwind/src/metro/processor/css.ts`:
- Around line 518-522: The isTransformArray method can throw if an element is
null because typeof null === 'object'; update the predicate in isTransformArray
to guard against null/undefined (e.g., check value != null or value !== null)
before checking 'type' and CSS.TRANSFORM_TYPES so values.every(...) only
accesses properties on non-null objects; keep the rest of the logic (using
CSS.TRANSFORM_TYPES.has(value.type)) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3049cdf0-a5a4-4f1c-a67a-a7e89a8daab6
📒 Files selected for processing (3)
packages/uniwind/src/metro/processor/css.tspackages/uniwind/tests/native/styles-parsing/transforms.test.tsxpackages/uniwind/tests/test.css
|
🚀 This pull request is included in v1.6.0. See Release v1.6.0 for release notes. |
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores