Skip to content

Wrap Recharts Sankey chart - #6708

Open
harsh21234i wants to merge 11 commits into
reflex-dev:mainfrom
harsh21234i:fix/wrap-recharts-sankey-6558
Open

Wrap Recharts Sankey chart#6708
harsh21234i wants to merge 11 commits into
reflex-dev:mainfrom
harsh21234i:fix/wrap-recharts-sankey-6558

Conversation

@harsh21234i

Copy link
Copy Markdown
Contributor

Closes #6558

Summary

  • Add rx.recharts.SankeyChart / rx.recharts.sankey_chart
  • Export Sankey through the Recharts package API
  • Add docs with simple, stateful, and custom typed/styled node examples
  • Add unit coverage for responsive-container wrapping and render alias

Tests

  • uv run pytest tests/units/components/graphing/test_recharts.py -q
  • uv run ruff check ...
  • uv run ruff format --check ...
  • uv run pyright ...

@harsh21234i
harsh21234i requested review from a team and Alek99 as code owners July 6, 2026 16:13
@greptile-apps

greptile-apps Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds rx.recharts.SankeyChart / rx.recharts.sankey_chart — a full Recharts Sankey diagram wrapper — along with rx.vars.use_hook_var() / rx.vars.use_id() helpers for binding React hook return values to Vars. All concerns from earlier review rounds (required name field, SankeyLinkPayload.source/target typed as SankeyNodePayload rather than int, dy not width, and get_type_hints-based deferred-annotation support) have been correctly addressed.

  • Sankey chart: SankeyChart inherits ChartBase (auto-wraps in ResponsiveContainer), exposes @sankey_chart.node / @sankey_chart.link decorators for custom SVG renderers, and uses memo + RestProp.to(TypedDict) to bridge Recharts' JS props into typed Reflex Vars.
  • Hook Vars: use_hook_var aliases each imported hook name per-call to prevent collisions when the same hook name appears in multiple libraries; use_id is a thin convenience wrapper over it.
  • Tests: Good coverage of responsive-container wrapping, Recharts runtime payload shape, deferred-annotation acceptance, positional-only parameter rejection, and hook-var VarData integrity.

Confidence Score: 5/5

Safe to merge — adds a well-isolated new component with no changes to existing chart logic and solid test coverage.

All previously raised correctness concerns have been addressed. The new SankeyChart, TypedDicts, and hook-var helpers are self-contained additions with no modifications to existing chart paths. The two observations are minor maintenance notes that do not affect runtime correctness.

Files Needing Attention: The pyproject.toml runtime dependency on reflex-components-core is worth a quick double-check to confirm it is intentional.

Important Files Changed

Filename Overview
packages/reflex-components-recharts/src/reflex_components_recharts/charts.py Adds SankeyChart, typed TypedDicts for nodes/links/props/payloads, @sankey_node/@sankey_link decorators with get_type_hints-based annotation resolution, and SankeyNamespace. All previously raised concerns (required name field, SankeyLinkPayload source/target as node objects, dy instead of width, deferred-annotation support) are correctly addressed.
packages/reflex-base/src/reflex_base/vars/special.py New use_hook_var / use_id helpers. Uses get_unique_variable_name to avoid name collisions, aliases each hook import to prevent same-name hook clashes across libraries, and correctly delegates type specialisation to Var.guess_type(). Implementation is clean and well-tested.
packages/reflex-components-recharts/src/reflex_components_recharts/general.py Adds use_chart_width() that wraps useChartWidth hook, and adds SankeyChart to ResponsiveContainer's _valid_children list (uses the Python class name, consistent with other charts).
tests/units/components/graphing/test_recharts.py Good coverage: ResponsiveContainer wrapping, alias rendering, namespace as style key, unannotated state data, SankeyLinkPayload shape match against Recharts runtime, deferred-annotation decorator acceptance, positional-only parameter rejection, and use_chart_width hook var data.
tests/units/reflex_base/vars/test_special.py New test file covering use_hook_var VarData, type specialisation, uniqueness, import aliasing for same-named hooks from different libraries, use_id, and hook-var hoisting into a component.
packages/reflex-components-recharts/pyproject.toml Adds reflex-components-core as a new runtime dependency and bumps reflex-base minimum to 0.9.7. No direct imports from reflex-components-core appear in the new Sankey code — worth verifying the runtime dep is required by existing (not new) code paths.
docs/library/graphing/charts/sankeychart.md New docs page covering simple, stateful, and fully-custom node/link rendering examples with correct type annotation patterns.

Reviews (10): Last reviewed commit: "Address Sankey docs and namespace feedba..." | Re-trigger Greptile

Comment thread packages/reflex-components-recharts/src/reflex_components_recharts/charts.py Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e94c1102db

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/reflex-components-recharts/src/reflex_components_recharts/charts.py Outdated
@codspeed-hq

codspeed-hq Bot commented Jul 9, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 26 untouched benchmarks
⏩ 8 skipped benchmarks1


Comparing harsh21234i:fix/wrap-recharts-sankey-6558 (6a09841) with main (a08a061)

Open in CodSpeed

Footnotes

  1. 8 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@masenf masenf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs a changelog entry.

doesn't build

TypeError: Invalid var passed for prop SankeyChart.data, expected type <class 
'reflex_components_recharts.charts.SankeyData'>, got value 
reflex___state____state___docgen_exec___docs_library_graphing_charts_sankeychart_md____sankey_state
.data_rx_state_ of type collections.abc.Mapping[str, 
collections.abc.Sequence[collections.abc.Mapping[str, int]] | 
collections.abc.Sequence[collections.abc.Mapping[str, str]]].
While rendering demo block in docs/library/graphing/charts/sankeychart.md:
class SankeyState(rx.State):
    data = {
        "nodes": [
            {"name": "Marketing"},
            {"name": "Trial"},
            {"name": "Sales"},
            {"name": "Support"},
         
Happened while evaluating page 'library/graphing/charts/sankeychart'

@harsh21234i

Copy link
Copy Markdown
Contributor Author

Removed link_width because Recharts does not expose a top-level Sankey prop for link width. Link width is
computed from link values and exposed only to custom link renderers.
Widened SankeyChart.data to accept mapping-shaped state vars as well as the explicit SankeyData TypedDict, so
the stateful docs example with unannotated state data works at component construction.
Added regression coverage for both the removed dead prop and unannotated state data.

The earlier SankeyNode.name and SankeyLink optional styling comments were already addressed in the current branch.

@harsh21234i
harsh21234i requested a review from masenf July 10, 2026 03:17
@harsh21234i

Copy link
Copy Markdown
Contributor Author

hey @masenf @FarhanAliRaza can someone look into this ?

@masenf masenf added the In Review Actively being reviewed by reflex team member label Jul 29, 2026
masenf added 3 commits July 29, 2026 15:41
* Implement use_hook_var, use_chart_width, and use_id
* Add memo-based decorator wrappers for `sankey_chart.node` and
  `sankey_chart.link` that do the needful to convert the decorated function
  into a custom component capable of being passed to sankey_chart as props.

Updated the docs to show a proper custom link+node example; removed broken
partial customization example (merged working pieces with the stateful example).
- Add reflex-base news entry for the new use_hook_var/use_id APIs (reflex-dev#6708)
- Extend recharts news entry to mention custom node/link renderers and
  use_chart_width
- Add unit tests for reflex_base.vars.special and rx.recharts.use_chart_width
- Document hook vars in the var_system API reference and wrapping-react
  custom-code-and-hooks pages; point to the helpers from the sankey docs

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QQXD9PDAbVVDieBfgSHiPX
masenf pushed a commit that referenced this pull request Jul 30, 2026
Addresses #6708 (comment)

Verified against the recharts@3.8.1 published types and runtime: the link
payload is {...link, source: sourceNode, target: targetNode}, so source and
target are resolved node payloads (not indices), thickness is dy (not width),
and index exists only on the outer SankeyLinkProps.

Also alias the Sankey TypedDicts at module level for the namespace
attributes: the generated .pyi rendered 'X = X' class attrs as
self-referential type aliases, which broke resolution of
sankey_chart.SankeyLinkProps and friends under pyright (7 stub errors, now 0).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QQXD9PDAbVVDieBfgSHiPX

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 issues found across 15 files

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="docs/library/graphing/charts/sankeychart.md">

<violation number="1" location="docs/library/graphing/charts/sankeychart.md:155">
P2: Custom node labels keep SVG's default black fill and only add a gray outline, so they become hard to read in dark mode. Use `fill` to set the intended text color instead of `stroke`.</violation>
</file>

<file name="packages/reflex-base/src/reflex_base/vars/special.py">

<violation number="1" location="packages/reflex-base/src/reflex_base/vars/special.py:9">
P3: Direct `use_hook_var(..., _var_type=int)` calls lose their static result type because the return annotation is bare `Var`, despite returning a type-specific var at runtime. A generic/overloaded signature that links `_var_type` to `Var[T]` would preserve editor and Pyright type checking for this public helper.</violation>
</file>

<file name="packages/reflex-components-recharts/src/reflex_components_recharts/charts.py">

<violation number="1" location="packages/reflex-components-recharts/src/reflex_components_recharts/charts.py:621">
P3: Minor dead-code/robustness note: `sig.parameters[next(...)] is None` can never be true (a `Parameter` object is never `None`), so this clause is dead. The empty-signature case is already excluded by the preceding `len(sig.parameters) != 1` check. It also uses index-style `next(iter(...))` into the mapping unnecessarily; iterating the values directly is clearer.</violation>

<violation number="2" location="packages/reflex-components-recharts/src/reflex_components_recharts/charts.py:622">
P2: The decorator's runtime validation compares the decorated function's raw parameter annotation with `get_origin(...) is not Var`. When the caller's module uses `from __future__ import annotations` (common in Reflex apps, and where this is enforced on many user modules), `inspect.signature` returns the annotation as an unevaluated string, so `get_origin` returns `None` and the check always fails — the decorator raises `TypeError` at import time for a fully valid `node: rx.Var[SankeyNodeProps]` function, breaking the documented custom-renderer API with a confusing error. Consider resolving annotations (e.g. `typing.get_type_hints(fn, include_extras=True)`) before validating, or skipping the eager check and letting the memo machinery handle typing, so both evaluated and string-lazy annotations are accepted. The same issue exists in `sankey_link`.</violation>

<violation number="3" location="packages/reflex-components-recharts/src/reflex_components_recharts/charts.py:630">
P3: A valid single positional-only renderer parameter passes validation but then fails during memo creation with an unexpected-keyword `TypeError`. Either reject unsupported parameter kinds during validation or invoke positional-only callbacks positionally in both decorators.</violation>

<violation number="4" location="packages/reflex-components-recharts/src/reflex_components_recharts/charts.py:637">
P3: The newly added `sankey_link` decorator duplicates nearly all of `sankey_node`, including validation that already needs identical fixes in two places. A shared renderer-decorator helper parameterized by props type/name would prevent these paths from drifting.</violation>
</file>

Tip: instead of fixing issues one by one fix them all with cubic

Re-trigger cubic

Comment thread docs/library/graphing/charts/sankeychart.md Outdated
Comment thread packages/reflex-components-recharts/src/reflex_components_recharts/charts.py Outdated
Comment thread packages/reflex-base/src/reflex_base/vars/special.py Outdated
Comment thread packages/reflex-components-recharts/src/reflex_components_recharts/charts.py Outdated
Comment thread packages/reflex-components-recharts/src/reflex_components_recharts/charts.py Outdated

harsh21234i commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Pushed ad5de4fa with two fixes:

  • Added the missing reflex-components-core dependency for reflex-components-recharts and its pyi build hook dependencies, which fixes the package build failure caused by reflex_base.components.memo importing reflex_components_core.
  • Updated the Sankey custom link payload types to match the Recharts runtime shape: payload.source / payload.target are resolved node payloads, link height uses dy, and index remains on the outer link props. Added a regression test for this shape.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7 issues found across 15 files

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/pyproject.toml">

<violation number="1" location="packages/reflex-components-recharts/pyproject.toml:11">
P1: This development-release requirement blocks the Recharts package publish gate and can make released metadata unresolvable for downstream users. Re-pin `reflex-base` to the released version containing the required API before publishing.</violation>
</file>

<file name="docs/library/graphing/charts/sankeychart.md">

<violation number="1" location="docs/library/graphing/charts/sankeychart.md:57">
P2: In the "Stateful Example", `SankeyState.data` is declared without a type annotation, so it is not a reactive Var (Reflex builds state vars from annotated fields). The `Randomize flows` button mutates that plain dict in place, which produces no backend state delta, so the chart will not update when clicked. Annotate `data` (e.g. `data: dict[str, Any]` or a `SankeyData`) so the mutation triggers a re-render and the example actually demonstrates the stateful behavior it claims.</violation>

<violation number="2" location="docs/library/graphing/charts/sankeychart.md:165">
P3: Link value labels retain the SVG default black fill, so they can become unreadable on dark backgrounds and render as outlined text elsewhere. This likely should use `fill=rx.color("gray", 12)` like the node labels.</violation>
</file>

<file name="packages/reflex-base/src/reflex_base/vars/special.py">

<violation number="1" location="packages/reflex-base/src/reflex_base/vars/special.py:45">
P2: Pages using same-named hooks from different libraries fail during import compilation because each hook is imported without a unique alias. Consider aliasing the imported hook symbol (and invoking that alias) so `use_hook_var` can safely compose across packages.</violation>
</file>

<file name="packages/reflex-components-recharts/src/reflex_components_recharts/charts.py">

<violation number="1" location="packages/reflex-components-recharts/src/reflex_components_recharts/charts.py:11">
P3: Importing `_MemoComponentWrapper` makes the Recharts package depend on a private `reflex-base` implementation detail, so an internal memo refactor can break this package at import time. A public callable/protocol return type would avoid exposing that coupling in the Sankey decorator API.</violation>

<violation number="2" location="packages/reflex-components-recharts/src/reflex_components_recharts/charts.py:527">
P3: Custom `name_key`/`data_key` datasets are rejected by the new `SankeyData` type because `name` and `value` remain required default-key fields. Marking those two fields `NotRequired` would let the TypedDict model the alternate-key props exposed by `SankeyChart`.</violation>

<violation number="3" location="packages/reflex-components-recharts/src/reflex_components_recharts/charts.py:739">
P2: Using `rx.recharts.sankey_chart` as a component-style key raises `TypeError: unhashable type: 'SankeyNamespace'`, and it is not normalized by `evaluate_style_namespaces`. Inheriting from `ComponentNamespace` would preserve the standard callable-namespace styling behavior.</violation>
</file>

Tip: instead of fixing issues one by one fix them all with cubic

Re-trigger cubic

requires-python = ">=3.10"
dependencies = ["reflex-base >= 0.9.0"]
dependencies = [
"reflex-base >= 0.9.7.post31.dev0",

@cubic-dev-ai cubic-dev-ai Bot Aug 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: This development-release requirement blocks the Recharts package publish gate and can make released metadata unresolvable for downstream users. Re-pin reflex-base to the released version containing the required API before publishing.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-components-recharts/pyproject.toml, line 11:

<comment>This development-release requirement blocks the Recharts package publish gate and can make released metadata unresolvable for downstream users. Re-pin `reflex-base` to the released version containing the required API before publishing.</comment>

<file context>
@@ -7,7 +7,10 @@ readme = "README.md"
 requires-python = ">=3.10"
-dependencies = ["reflex-base >= 0.9.0"]
+dependencies = [
+    "reflex-base >= 0.9.7.post31.dev0",
+    "reflex-components-core >= 0.9.0",
+]
</file context>
Fix with cubic

var_name := get_unique_variable_name(),
_var_type=cast(GenericType, _var_type),
_var_data=VarData(
imports={library: hook},

@cubic-dev-ai cubic-dev-ai Bot Aug 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Pages using same-named hooks from different libraries fail during import compilation because each hook is imported without a unique alias. Consider aliasing the imported hook symbol (and invoking that alias) so use_hook_var can safely compose across packages.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-base/src/reflex_base/vars/special.py, line 45:

<comment>Pages using same-named hooks from different libraries fail during import compilation because each hook is imported without a unique alias. Consider aliasing the imported hook symbol (and invoking that alias) so `use_hook_var` can safely compose across packages.</comment>

<file context>
@@ -0,0 +1,57 @@
+        var_name := get_unique_variable_name(),
+        _var_type=cast(GenericType, _var_type),
+        _var_data=VarData(
+            imports={library: hook},
+            hooks=(f"const {var_name} = {hook}();",),
+        ),
</file context>
Fix with cubic

Comment thread packages/reflex-components-recharts/src/reflex_components_recharts/charts.py Outdated
Comment thread docs/library/graphing/charts/sankeychart.md
Comment thread docs/library/graphing/charts/sankeychart.md
Comment thread packages/reflex-components-recharts/src/reflex_components_recharts/charts.py Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 15 files

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="docs/library/graphing/charts/sankeychart.md">

<violation number="1" location="docs/library/graphing/charts/sankeychart.md:151">
P3: Link value labels are overpainted by the wide Sankey path, reducing or eliminating their readability for high-volume links. Rendering the path before `rx.el.svg.text` keeps the label on top.</violation>
</file>

<file name="packages/reflex-base/src/reflex_base/vars/special.py">

<violation number="1" location="packages/reflex-base/src/reflex_base/vars/special.py:26">
P3: PEP 604 calls such as `_var_type=int | None` are exposed as `Var[Unknown]` because this overload returns a raw generic. If the union members cannot currently be inferred by Pyright, declaring `Var[Any]` explicitly would at least prevent an unknown type from leaking through this public API.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

# Determine if the node is at the right edge of the chart to adjust the label position accordingly.
is_out = node.x + node.width + 6 > rx.recharts.use_chart_width()
return rx.fragment(
rx.el.svg.text(

@cubic-dev-ai cubic-dev-ai Bot Aug 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Link value labels are overpainted by the wide Sankey path, reducing or eliminating their readability for high-volume links. Rendering the path before rx.el.svg.text keeps the label on top.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/library/graphing/charts/sankeychart.md, line 151:

<comment>Link value labels are overpainted by the wide Sankey path, reducing or eliminating their readability for high-volume links. Rendering the path before `rx.el.svg.text` keeps the label on top.</comment>

<file context>
@@ -0,0 +1,221 @@
+        # Determine if the node is at the right edge of the chart to adjust the label position accordingly.
+        is_out = node.x + node.width + 6 > rx.recharts.use_chart_width()
+        return rx.fragment(
+            rx.el.svg.text(
+                node.payload.name,
+                x=rx.cond(is_out, node.x - 6, node.x + node.width + 6),
</file context>
Fix with cubic



@overload
def use_hook_var(library: str, hook: str, _var_type: UnionType) -> Var: ...

@cubic-dev-ai cubic-dev-ai Bot Aug 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: PEP 604 calls such as _var_type=int | None are exposed as Var[Unknown] because this overload returns a raw generic. If the union members cannot currently be inferred by Pyright, declaring Var[Any] explicitly would at least prevent an unknown type from leaking through this public API.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-base/src/reflex_base/vars/special.py, line 26:

<comment>PEP 604 calls such as `_var_type=int | None` are exposed as `Var[Unknown]` because this overload returns a raw generic. If the union members cannot currently be inferred by Pyright, declaring `Var[Any]` explicitly would at least prevent an unknown type from leaking through this public API.</comment>

<file context>
@@ -0,0 +1,60 @@
+
+
+@overload
+def use_hook_var(library: str, hook: str, _var_type: UnionType) -> Var: ...
+
+
</file context>
Fix with cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

In Review Actively being reviewed by reflex team member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wrap recharts SankeyChart

3 participants