fix: handle multiline classes - #610
Conversation
📝 WalkthroughWalkthroughNative style resolution now handles newline- and whitespace-separated class names. A new test verifies multiline classes produce the expected background color and border width. ChangesMultiline className parsing
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR adds multiline class-name support to native style resolution. The main changes are:
Confidence Score: 4/5The changed native tokenization path needs a small fix before merging.
packages/uniwind/src/core/native/store.ts Important Files Changed
Reviews (1): Last reviewed commit: "fix: handle multiline classes" | Re-trigger Greptile |
| const classNameTokens = classNames.includes('\n') | ||
| ? classNames.split(/\s+/).filter(Boolean) | ||
| : classNames.split(' ') |
There was a problem hiding this comment.
Non-Newline Whitespace Joins Classes
When a native className uses tabs without a newline, such as bg-red-500\tborder-2, this condition selects the space-only split and treats both classes as one unknown token. Native rendering then silently omits both styles, while the web resolver accepts the same whitespace-separated input.
| const classNameTokens = classNames.includes('\n') | |
| ? classNames.split(/\s+/).filter(Boolean) | |
| : classNames.split(' ') | |
| const classNameTokens = classNames.split(/\s+/).filter(Boolean) |
There was a problem hiding this comment.
This will resolve in performance drop
There was a problem hiding this comment.
That's a fair point — split(/\s+/).filter(Boolean) is measurably slower than split(' ') on the hot path. Given that the PR already benchmarks the overhead carefully, always using the regex split would undermine that work.
A middle ground: extend the fast-path condition to also cover tabs, keeping the regex split only for non-space whitespace:
| const classNameTokens = classNames.includes('\n') | |
| ? classNames.split(/\s+/).filter(Boolean) | |
| : classNames.split(' ') | |
| const classNameTokens = classNames.includes('\n') || classNames.includes('\t') | |
| ? classNames.split(/\s+/).filter(Boolean) | |
| : classNames.split(' ') |
This preserves the fast split(' ') path for the common case while still correctly handling tab-separated class names. If tabs in className strings are genuinely not a concern in your usage (i.e. always space- or newline-separated), then the original code is fine as-is.
Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/uniwind/src/core/native/store.ts`:
- Around line 111-115: Update the classNameTokens initialization in the
class-name parsing flow to always split classNames with the existing whitespace
regex and remove the conditional split(' ') fallback, preserving the
filter(Boolean) behavior so spaces, tabs, newlines, and other whitespace
separators produce individual class names.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 244c4130-4b74-41f7-b002-b12506555074
📒 Files selected for processing (2)
packages/uniwind/src/core/native/store.tspackages/uniwind/tests/native/styles-parsing/multiline.test.tsx
#608
Adds only around ~2-3ms in our benchmarks
Summary by CodeRabbit