fix(recharts): stop routing component props to wrapperStyle - #6833
fix(recharts): stop routing component props to wrapperStyle#6833YoussefMohamed2k19 wants to merge 4 commits into
Conversation
Declare stroke_dasharray on ReferenceLine and tick_formatter on Axis (shared by XAxis/YAxis) as explicit fields. Previously undeclared kwargs fell through Component.create()'s default classification into style, which _get_style() (added in reflex-dev#4447) then dumps into wrapperStyle, so the props never reached the underlying Recharts component. Fixes reflex-dev#6575
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Greptile SummaryThis PR declares Recharts axis formatters and reference-line dash patterns as explicit component fields so they are emitted as component props rather than wrapper styles.
Confidence Score: 4/5The PR is not yet safe to merge because reactive string Vars remain accepted for The literal-string path from the earlier report is now converted to a function expression, but the existing reactive-formatter finding remains: an ordinary state-backed Files Needing Attention: packages/reflex-components-recharts/src/reflex_components_recharts/cartesian.py
|
| Filename | Overview |
|---|---|
| packages/reflex-components-recharts/src/reflex_components_recharts/cartesian.py | Declares the missing Recharts props and adds formatter normalization during axis creation. |
| tests/units/components/recharts/test_cartesian.py | Adds rendering and validation regression tests for the newly declared component props. |
| pyi_hashes.json | Updates the generated cartesian stub hash for the component API changes. |
| news/6575.bugfix.md | Documents the corrected routing of Recharts props. |
Reviews (4): Last reviewed commit: "fix(recharts): reject plain Python calla..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…tring Addresses review feedback on reflex-dev#6833: - tick_formatter was declared Var[str], so a plain Python string got wrapped by LiteralVar into a JSON-quoted string literal. Recharts received "(value) => value" as text, not a callable, so the formatter was silently never invoked. Axis.create() now wraps a str value in FunctionStringVar so it renders as raw, unquoted JS. - Tests previously only checked that the prop key existed, which would still pass for an empty/dropped value. They now assert the exact rendered prop string, and cover YAxis (inherits from Axis same as XAxis) in addition to XAxis.
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Addresses two more review comments on reflex-dev#6833: - tick_formatter was Var[Any] after the previous fix, silently accepting any type (int, list, ...) and only failing at the Recharts/JS layer with a garbled prop. Narrowed to Var[str | Callable[..., Any]] so non-callable, non-string values are rejected with a TypeError at component-creation time, both at runtime and in the generated .pyi stubs. (A first attempt using Var[FunctionVar]/Var[ReflexCallable[Any, Any]] hit a framework quirk where typehint_issubclass compares two independently constructed Protocol generic aliases by identity rather than structural equality, and intermittently rejected the exact value it just created — collections.abc.Callable doesn't hit that path.) - Doc string said "raw JS function body" but the example (and the actual behavior) is a full function expression, e.g. "(value) => value.toFixed(2)", not just a body like "return value". Reworded to "function expression" to match.
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
Addresses review feedback on reflex-dev#6833: tick_formatter=lambda value: value passed the declared/runtime type check (a lambda IS a collections.abc.Callable instance), but only strings were converted to a JS-function Var in create(). The raw Python lambda sat unconverted on the component, then blew up at render() with a cryptic "Unsupported type <class 'function'> for LiteralVar" error instead of a clear message at creation time. create() now explicitly rejects any non-str, non-Var tick_formatter (covering lambdas, named functions, and other Python objects) with a TypeError up front. Also normalizes an already-Var-wrapped FunctionVar's _var_type so passing e.g. FunctionStringVar.create("someGlobalFn") directly still works regardless of how the caller built it.
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/reflex-components-recharts/src/reflex_components_recharts/cartesian.py">
<violation number="1" location="packages/reflex-components-recharts/src/reflex_components_recharts/cartesian.py:152">
P2: A reactive string such as `State.formatter` still reaches Recharts as `tickFormatter={state.formatter}`, where Recharts expects a function. The `Var` branch should validate a callable `_var_type` instead of allowing every `Var`.</violation>
<violation number="2" location="packages/reflex-components-recharts/src/reflex_components_recharts/cartesian.py:157">
P3: Type checkers still accept the Python callables this branch now rejects at runtime because the generated `create` signature includes bare `Callable`. An explicit typed `tick_formatter` parameter/overload should expose only raw `str` and function-valued `Var` inputs.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| props["tick_formatter"] = tick_formatter._replace( | ||
| _var_type=Callable[..., Any] # pyright: ignore [reportArgumentType] | ||
| ) | ||
| elif not isinstance(tick_formatter, Var): |
There was a problem hiding this comment.
P2: A reactive string such as State.formatter still reaches Recharts as tickFormatter={state.formatter}, where Recharts expects a function. The Var branch should validate a callable _var_type instead of allowing every Var.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-components-recharts/src/reflex_components_recharts/cartesian.py, line 152:
<comment>A reactive string such as `State.formatter` still reaches Recharts as `tickFormatter={state.formatter}`, where Recharts expects a function. The `Var` branch should validate a callable `_var_type` instead of allowing every `Var`.</comment>
<file context>
@@ -136,11 +136,28 @@ def create(cls, *children, **props):
+ props["tick_formatter"] = tick_formatter._replace(
+ _var_type=Callable[..., Any] # pyright: ignore [reportArgumentType]
+ )
+ elif not isinstance(tick_formatter, Var):
+ msg = (
+ "tick_formatter must be a raw JS function expression string "
</file context>
| "tick_formatter must be a raw JS function expression string " | ||
| f'(e.g. "(value) => value.toFixed(2)") or a Var, got a Python ' | ||
| f"{type(tick_formatter).__name__}. Python values (including " | ||
| "plain callables like lambdas) cannot be sent to the client " |
There was a problem hiding this comment.
P3: Type checkers still accept the Python callables this branch now rejects at runtime because the generated create signature includes bare Callable. An explicit typed tick_formatter parameter/overload should expose only raw str and function-valued Var inputs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-components-recharts/src/reflex_components_recharts/cartesian.py, line 157:
<comment>Type checkers still accept the Python callables this branch now rejects at runtime because the generated `create` signature includes bare `Callable`. An explicit typed `tick_formatter` parameter/overload should expose only raw `str` and function-valued `Var` inputs.</comment>
<file context>
@@ -136,11 +136,28 @@ def create(cls, *children, **props):
+ "tick_formatter must be a raw JS function expression string "
+ f'(e.g. "(value) => value.toFixed(2)") or a Var, got a Python '
+ f"{type(tick_formatter).__name__}. Python values (including "
+ "plain callables like lambdas) cannot be sent to the client "
+ "as-is and are not supported."
+ )
</file context>
1 participant
Declare stroke_dasharray on ReferenceLine and tick_formatter on Axis (shared by XAxis/YAxis) as explicit fields. Previously undeclared kwargs fell through Component.create()'s default classification into style, which _get_style() (added in #4447) then dumps into wrapperStyle, so the props never reached the underlying Recharts component.
Fixes #6575
All Submissions:
Type of change
Changes To Core Features:
What changed
Component.create()treats any kwarg not declared as an explicit field on the class as a style prop._get_style()(added in #4447) then dumpsstyleintowrapperStyle.stroke_dasharrayonReferenceLineandtick_formatteronXAxis/YAxisweren't declared as real fields, so they were misclassified as CSS and never reached the underlying Recharts component — dashed reference lines rendered solid,tick_formatterwas ignored.Fix: declared both as explicit
Varfields onReferenceLineand the sharedAxisbase class.Test plan
style/wrapperStyle(tests/units/components/recharts/test_cartesian.py)uv run pytest tests/units/components/recharts— 10 passeduv run ruff check ./ruff format --check ./uv run pyright— clean.pyistubs regenerated viascripts/make_pyi.pypre-commit run --files ...— all hooks passedcloses #6575