Fix: CSS - Work comma selector with parens#199
Conversation
🦋 Changeset detectedLatest commit: 1c89ca9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis update introduces a bug fix for the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant complexSelectors
Caller->>complexSelectors: nestedSelectorKey(selector)
complexSelectors->>complexSelectors: splitSelector(selector)
complexSelectors-->>Caller: Correctly split selectors (respecting parentheses/brackets)
Possibly related PRs
Suggested labels
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
yarn install v1.22.22 ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Triggered from #199 by @black7375. Checking if we can fast forward Target branch ( commit cdbc5a3e25b2cc857c36d9814e6c7584e93ff5ab (HEAD -> main, tag: @mincho-js/transform-to-vanilla@0.2.1, tag: @mincho-js/react@0.1.1, tag: @mincho-js/css@0.2.1, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Wed Apr 23 03:25:29 2025 +0900
Chore: Add cspell allow wordsPull request ( commit 1c89ca943c9d1495230145d47cf810d820aeddbb (pull_request/nested-comma-selector)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Fri Apr 25 02:03:20 2025 +0900
Fix: CSS - Work comma selector with parensIt is possible to fast forward |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/transform-to-vanilla/src/transform-keys/complex-selectors.ts (2)
32-82: Well-implemented selector parsing logicThe
splitSelectorfunction correctly tracks nesting levels of parentheses and brackets to only split on commas at the root level. This approach properly handles complex CSS selectors containing commas within pseudo-classes like:not(:active, :disabled)or attribute selectors.I have a couple of minor suggestions to improve readability and robustness:
function splitSelector(selector: string): string[] { if (!selector.includes(",")) { return [selector]; } const result = []; let currentSelector = ""; + // Track nesting level of parentheses and brackets let parenLevel = 0; let bracketLevel = 0; const selectorLength = selector.length; for (let i = 0; i < selectorLength; i++) { const char = selector[i]; switch (char) { case "(": parenLevel++; currentSelector += char; break; case ")": parenLevel--; + // Optional: Handle unbalanced parentheses + if (parenLevel < 0) parenLevel = 0; currentSelector += char; break; case "[": bracketLevel++; currentSelector += char; break; case "]": bracketLevel--; + // Optional: Handle unbalanced brackets + if (bracketLevel < 0) bracketLevel = 0; currentSelector += char; break; case ",": if (parenLevel === 0 && bracketLevel === 0) { result.push(currentSelector); currentSelector = ""; } else { currentSelector += char; } break; default: currentSelector += char; break; } } if (currentSelector.trim() !== "") { result.push(currentSelector); } return result; }
144-155: Excellent test coverage for complex selectorsThe added test thoroughly verifies the handling of complex nested selectors with commas inside parentheses and brackets. This is a good addition that helps ensure the code is working correctly.
You might consider adding one more test case for edge cases like empty selectors or selectors with escaped commas, but this is comprehensive enough for the current fix.
.changeset/twenty-laws-create.md (1)
1-6: Good changeset documentationThe changeset correctly documents the patch with a clear description of the bug being fixed.
Consider using a proper Markdown heading instead of emphasis for better formatting:
--- "@mincho-js/transform-to-vanilla": patch --- -**Nested Selector with commas and parens** +## Nested Selector with commas and parens - Fixes an error that occurs when parentheses are present.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
5-5: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/twenty-laws-create.md(1 hunks)packages/transform-to-vanilla/src/transform-keys/complex-selectors.ts(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/transform-to-vanilla/src/transform-keys/complex-selectors.ts (2)
packages/transform-to-vanilla/src/transform-object/index.ts (2)
TransformContext(58-68)initTransformContext(69-82)packages/transform-to-vanilla/src/index.ts (2)
TransformContext(4-4)initTransformContext(3-3)
🪛 markdownlint-cli2 (0.17.2)
.changeset/twenty-laws-create.md
5-5: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
packages/transform-to-vanilla/src/transform-keys/complex-selectors.ts (1)
19-19: Good improvement to handle complex selectorsReplacing the naive comma split with a specialized
splitSelectorfunction will properly handle commas within parentheses and brackets, fixing the issue mentioned in the PR description.
|
/fast-forward |
|
Triggered from #199 (comment) by @black7375. Trying to fast forward Target branch ( commit cdbc5a3e25b2cc857c36d9814e6c7584e93ff5ab (HEAD -> main, tag: @mincho-js/transform-to-vanilla@0.2.1, tag: @mincho-js/react@0.1.1, tag: @mincho-js/css@0.2.1, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Wed Apr 23 03:25:29 2025 +0900
Chore: Add cspell allow wordsPull request ( commit 1c89ca943c9d1495230145d47cf810d820aeddbb (pull_request/nested-comma-selector)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Fri Apr 25 02:03:20 2025 +0900
Fix: CSS - Work comma selector with parensFast forwarding $ git push origin 1c89ca943c9d1495230145d47cf810d820aeddbb:main
To https://github.com/mincho-js/mincho.git
cdbc5a3..1c89ca9 1c89ca943c9d1495230145d47cf810d820aeddbb -> main |
1 participant
Description
Fix at paren case.
Related Issue
Summary by CodeRabbit
Bug Fixes
Tests
Additional context
Checklist