fix issubclass calls#5753
Conversation
There was a problem hiding this comment.
Greptile Summary
This PR fixes regressions related to issubclass calls that were causing TypeError exceptions when working with certain type annotations. The changes address two specific areas where unsafe type checking was occurring:
Component Property Detection (reflex/components/component.py):
The PR removes an optimization in the Component class that attempted to cache component property names using issubclass(field_type, Component) checks. This optimization was causing issues when field types weren't fully resolved or were complex type annotations. The code now falls back to a simpler approach that directly iterates through all props and checks each value at runtime, eliminating the problematic type introspection.
Literal Type Handling (reflex/utils/types.py):
The PR adds proper handling for Python's Literal types in the is_valid_var_type function. Previously, when Literal types (like Literal['a', 'b']) were passed to issubclass(), it would raise a TypeError since Literal types are not actual classes. The fix extracts the actual types from the literal values (e.g., extracting str from string literals) and validates those recursively.
These changes integrate well with the existing codebase by following established patterns - the Literal handling mirrors the existing Union type handling approach, and the component property detection falls back to the reliable runtime checking that was already in place as a fallback mechanism.
Confidence score: 4/5
- This PR is safe to merge with minimal risk of introducing new issues
- Score reflects targeted fixes for well-documented regressions with clear error patterns
- Pay attention to performance implications in component property detection as noted by the developer
2 files reviewed, 2 comments
CodSpeed Performance ReportMerging #5753 will not alter performanceComparing Summary
|
this should fix regressions as in: #5747 (comment)_
there's performance implications in the change in
components.py, will be looking at those as well