fix: improve invalid color error messages for literal color props#6433
fix: improve invalid color error messages for literal color props#6433BABTUNA wants to merge 5 commits intoreflex-dev:mainfrom
Conversation
Validate literal CSS color strings with clearer errors for color-like style keys and typed component color props (e.g. Var[str | Color]). Adds regressions for invalid style color values, valid color formats, and typed color prop rejection.
Greptile SummaryThis PR adds early-stage validation for literal CSS color string values, both in the Confidence Score: 4/5Safe to merge; all findings are style suggestions only, no logic bugs identified. All review findings are P2 (style/maintainability). The core validation logic is correct, test coverage is solid, and the component.py refactor is a net improvement over the original code. packages/reflex-base/src/reflex_base/style.py — three duplicate frozensets and the permissive CSS-function suffix check are worth a second look before this pattern is extended. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Style dict or Component prop assignment] --> B{Is value a plain str?}
B -- No --> Z[Pass through unchanged]
B -- Yes --> C{Is key a color style key?}
C -- No --> Z
C -- Yes --> D[_is_valid_css_color_value]
D --> E{Contains REFLEX_VAR tag?}
E -- Yes --> OK[Valid]
E -- No --> F{Ends with important?}
F -- Yes --> G[Strip suffix, re-check]
F -- No --> H{Is none?}
G --> H
H -- Yes --> I{fill or stroke key?}
I -- Yes --> OK
I -- No --> ERR[ValueError raised]
H -- No --> J{context-fill or context-stroke?}
J -- Yes --> K{fill or stroke key?}
K -- Yes --> OK
K -- No --> ERR
J -- No --> L{Named CSS color keyword?}
L -- Yes --> OK
L -- No --> M{Hex color format?}
M -- Yes --> OK
M -- No --> N{url prefix and fill or stroke key?}
N -- Yes --> OK
N -- No --> O{CSS color function prefix and ends with paren?}
O -- Yes --> OK
O -- No --> ERR
Reviews (1): Last reviewed commit: "fix: improve invalid color errors for li..." | Re-trigger Greptile |
| _CSS_URL_COLOR_KEYS = frozenset({ | ||
| "fill", | ||
| "stroke", | ||
| }) | ||
|
|
||
| _CSS_NONE_COLOR_KEYS = frozenset({ | ||
| "fill", | ||
| "stroke", | ||
| }) | ||
|
|
||
| _CSS_CONTEXT_PAINT_KEYS = frozenset({ | ||
| "fill", | ||
| "stroke", | ||
| }) |
There was a problem hiding this comment.
Three identical frozensets for SVG paint keys
_CSS_URL_COLOR_KEYS, _CSS_NONE_COLOR_KEYS, and _CSS_CONTEXT_PAINT_KEYS are all frozenset({"fill", "stroke"}). If a new SVG color property is added later (e.g., marker-start), all three must be updated in sync — a future maintainer may update only one and silently break the other two checks. A single shared constant would make the relationship explicit.
# All three SVG paint-specific key sets currently share the same members.
_SVG_PAINT_KEYS = frozenset({"fill", "stroke"})
_CSS_URL_COLOR_KEYS = _SVG_PAINT_KEYS
_CSS_NONE_COLOR_KEYS = _SVG_PAINT_KEYS
_CSS_CONTEXT_PAINT_KEYS = _SVG_PAINT_KEYSThere was a problem hiding this comment.
ok I'll clean it up a bit
| return lower.endswith(")") and any( | ||
| lower.startswith(prefix) | ||
| for prefix in _CSS_COLOR_FUNCTION_PREFIXES + _CSS_COLOR_VALUE_PREFIXES | ||
| ) |
There was a problem hiding this comment.
CSS function prefix check doesn't guard the closing parenthesis position
The check lower.endswith(")") and any(lower.startswith(prefix) ...) accepts structurally odd strings like "rgb(foo) bar()" where the closing paren is not the direct closing paren of the function. This is a known limitation of not doing full CSS parsing. If future stricter validation is added, this is the line to revisit. At minimum, a brief comment documenting this intentional trade-off would help future maintainers.
There was a problem hiding this comment.
this is intentionally permissive. Will add comment
Resolve component.py conflict and keep literal color validation logic. Also apply review cleanup: share SVG paint key constants and document permissive function-prefix parsing check in style validation.
Merging this PR will not alter performance
Comparing Footnotes
|
Summary
Color(for exampleVar[str | Color])Validation
uv run ruff check packages/reflex-base/src/reflex_base/style.py packages/reflex-base/src/reflex_base/components/component.py tests/units/test_style.py tests/units/components/test_component.pyuv run pytest tests/units/test_style.py tests/units/components/test_component.py -k "invalid_typed_color_prop_raises_error or invalid_literal_color_style_value_raises_error or valid_literal_color_style_values_are_accepted" -qCloses #4955