fix: correct regex operator precedence in CSS_LENGTH_UNIT_REGEX#7068
Conversation
The alternation `^px|cm|...|Q$` only anchored `px` at the start and `Q` at the end. Wrapping in a group `^(px|cm|...|Q)$` ensures the full match is properly anchored. Made-with: Cursor
WalkthroughModified the CSS_LENGTH_UNIT_REGEX pattern in the ReduceCSSCalc utility to enforce full-string matching by adding start and end anchors with grouped alternation. This prevents unintended partial matches within longer strings while maintaining all existing functional logic. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 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)
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)
src/util/ReduceCSSCalc.ts (1)
5-5: Please add/confirm a regression test for exact unit-only matches.This change should be covered with cases like valid (
px,vmin,%) and invalid (apx,pxx,xQ) to lock in the precedence fix.As per coding guidelines, "
src/**/*.{ts,tsx}: Aim for 100% unit test code coverage when writing new code" and "src/**/util/**/*.{ts,tsx}: Extract pure helper functions for data processing and unit test them".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/util/ReduceCSSCalc.ts` at line 5, Add a regression unit test for the CSS_LENGTH_UNIT_REGEX in ReduceCSSCalc that asserts exact unit-only matches and rejects similar but invalid strings: verify valid matches like "px", "vmin", "%" and invalid non-matches like "apx", "pxx", "xQ"; use the same matching logic the code uses (e.g., RegExp.prototype.test or full-string match) against the exported CSS_LENGTH_UNIT_REGEX from ReduceCSSCalc and include these cases so the precedence/anchoring fix is locked in.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/util/ReduceCSSCalc.ts`:
- Line 5: Add a regression unit test for the CSS_LENGTH_UNIT_REGEX in
ReduceCSSCalc that asserts exact unit-only matches and rejects similar but
invalid strings: verify valid matches like "px", "vmin", "%" and invalid
non-matches like "apx", "pxx", "xQ"; use the same matching logic the code uses
(e.g., RegExp.prototype.test or full-string match) against the exported
CSS_LENGTH_UNIT_REGEX from ReduceCSSCalc and include these cases so the
precedence/anchoring fix is locked in.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7068 +/- ##
=======================================
Coverage 90.20% 90.20%
=======================================
Files 526 526
Lines 39650 39650
Branches 5442 5442
=======================================
Hits 35768 35768
Misses 3873 3873
Partials 9 9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
CSS_LENGTH_UNIT_REGEXinsrc/util/ReduceCSSCalc.tshas a regex operator precedence bug.The current pattern:
Due to
|(alternation) having lower precedence than^and$, this only anchorspxat the start andQat the end — the middle alternatives are unanchored.Fix
Wrap the alternation in a non-capturing group so the anchors apply to the entire expression:
Impact
In practice this doesn't cause runtime issues because the regex is only tested against short, pre-parsed CSS unit strings. However, the current pattern triggers misleading-regex-precedence warnings in static analysis tools (ESLint, SonarQube), which propagate to downstream consumers of recharts.
Ref: elementary-data/elementary#2036
Made with Cursor
Summary by CodeRabbit
Bug Fixes