fix(dataeditor): apply default column type and id to reactive columns by anishgurjar · Pull Request #7597 · reflex-dev/reflex

anishgurjar · 2026-10-18T21:52:10Z

Summary

DataEditor.create only applies the documented default column type ("str") to literal column lists. Columns passed as a reactive state Var skip that branch and reach Glide DataEditor without a type, producing an invalid TextCell and crashing at render. Untyped reactive columns also reach Glide without an id, so dict-row data hits the same crash.

This applies the defaults at render time for reactive columns: the columns expression is wrapped so each entry is spread over { type: "str", id: (col.title ?? "").toLowerCase() }, so an explicit per-column type or id still wins (same semantics as the literal path via format_data_editor_column). The wrap uses Var._replace, which preserves the original var's _var_data, so state subscriptions and imports are unchanged.

Alternative considered: patching the default branch of formatCell in .templates/web/utils/helpers/dataeditor.js. That would only stop the TextCell crash at render; the python-side wrap normalizes the column definition itself, which keeps the default co-located with the literal path and also fixes dict-row lookup (id). Note the wrap produces a fresh columns array identity on each render (.map over the reactive value); Glide already re-renders on reactive prop changes, so no memoization contract changes.

Fixes #7596

Type of change

Bug fix (non-breaking change which fixes an issue)

Testing

test_dataeditor_reactive_columns_default_type: reactive Var columns render as (state.columns).map((col) => ({ type: "str", id: (col.title ?? "").toLowerCase(), ...col })) (columns prop looked up by name).
test_dataeditor_reactive_columns_preserve_var_data: rx.State-backed test pinning that the wrap preserves _var_data (state dependency tracked).
test_dataeditor_reactive_columns_default_id_supports_dict_rows: dict-row data renders without crashing.
test_dataeditor_reactive_columns_js_semantics: node-checked JS semantics - default applies, explicit type/id override wins.
test_dataeditor_literal_columns_default_type: pins existing literal-path behavior.
New tests fail without the fix; pytest tests/units/components/datadisplay/test_dataeditor.py: 8 passed. pyright: 0 errors. ruff check / ruff format --check: clean.
Full pytest tests/units: 12746 passed; 249 failures are all in reflex_cli/v2 (deploy/auth/gcp cloud tests needing credentials/network) plus one tty frontend test, none import or touch dataeditor.

cc @masenf

cubic-dev-ai

All reported issues were addressed across 3 files

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

View guided diff | Re-trigger cubic}

greptile-apps · 2026-10-18T21:57:33Z

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge, with one non-blocking test coverage gap remaining from the previous review.

Findings

Tests skip the cell callback ▶

Summary

Applies default type and id values to reactive DataEditor columns while preserving explicit values.

Reactive DataEditor columns get the defaults their cells need.

_{Reviews (3) · Last reviewed commit: "chore: refresh DataEditor generated stub..." · Reviewed by Greptile}

greptile-apps · 2026-10-18T21:57:36Z

+def test_dataeditor_reactive_columns_js_semantics():
+    test_script = """
+const cols = [{ title: "Name" }, { title: "Age", type: "int" }].map(
+  (col) => ({ type: "str", id: (col.title ?? "").toLowerCase(), ...col })
+);
+const row = { name: "Alice" };
+console.log(JSON.stringify([cols, row[cols[0].id]]));
+"""


Tests skip the cell callback

The dict-row test only checks the prop text. This JavaScript test runs a hand-written copy of the map and reads the row directly. Neither checks that the generated cell callback returns a valid cell through formatDataEditorCells, so both can pass if that callback stops using the normalized columns.

Execute the generated expression with the real helper, including a column whose explicit id differs from its lowercased title.

codspeed · 2026-10-19T06:33:34Z

Merging this PR will not alter performance

⚠️

21 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

test_get_state[memory]

test_cli_startup[version]

test_cli_startup[help]

test_import_cli

test_state_manager_memory_read_only_modify

test_get_state[disk]

test_state_manager_memory_modify

test_state_manager_memory_cold_get

test_yield_intermediate_update

test_modify_state[disk]

…and 11 more

✅ 148 untouched benchmarks

_{Comparing anishgurjar:fix-dataeditor-reactive-columns-default-type (495ec4f) with main (75a08e1)}

FarhanAliRaza

Can you please fix the review posted.

FarhanAliRaza · 2026-10-19T08:05:02Z

+            props["columns"] = columns._replace(
+                _js_expr=(
+                    f'({columns!s}).map((col) => ({{ type: "str", '
+                    f'id: (col.title ?? "").toLowerCase(), ...col }}))'
+                )
+            )


add_hooks builds the cell callback from str(self.columns). That is now this wrapped expression. So the compiled getData_* runs the .map() on every cell call:

return formatDataEditorCells(col, row, (state.cols).map((col) => ({ type: "str", id: ..., ...col })), state.data);

Glide calls this callback for each visible cell on each draw. So one draw rebuilds all column objects once per cell. In a Node benchmark with 40 rows, the extra cost per draw goes from 0.02 ms to 0.45 ms at 20 columns, and to 2.5 ms at 50 columns.

Please compute the wrapped columns once per render. Then use that one value in the columns prop and in getData.

FarhanAliRaza · 2026-10-19T08:05:02Z

+            # Reactive columns skip the literal formatting above, so apply the
+            # documented default column type when rendering (#7596). An
+            # explicit per-column type still wins via the spread order.


This comment is not accurate. The literal formatting is in the elif below, not above. The code also sets a default id, not only a default type. Please make the comment state what the code does.

FarhanAliRaza · 2026-10-19T08:05:02Z

+    wrapper = DataEditor.create(columns=EditorState.columns)
+    editor = cast(DataEditor, wrapper.children[0])
+    # The wrapped expression must keep the original var's data (state dep).
+    assert editor.columns._var_data == EditorState.columns._var_data


This line adds 2 pyright errors. The DataEditor stub has no columns attribute. The field type is a list, which has no _var_data. The same file on main has 0 pyright errors. Please make uv run pyright clean for this file.

FarhanAliRaza · 2026-10-19T08:05:02Z

+def test_dataeditor_reactive_columns_default_id_supports_dict_rows():
+    # Dict rows are keyed by column id; the JS-side id default keeps them
+    # working for reactive columns that omit id. See issue #7596.
+    editor = DataEditor.create(
+        columns=Var(_js_expr="state.columns"),
+        data=[{"name": "Alice"}],
+    ).render()
+    assert 'id: (col.title ?? "").toLowerCase()' in _columns_prop(editor)


This test asserts the same string as test_dataeditor_reactive_columns_default_type. The data argument has no effect on the assertion, so nothing here exercises dict rows. Line 142 also adds a pyright error: list[dict[str, str]] does not match Sequence[Sequence[Any]].

Please remove this test, or move the dict-row lookup into the Node test below.

FarhanAliRaza · 2026-10-19T08:05:02Z

+    assert 'id: (col.title ?? "").toLowerCase()' in _columns_prop(editor)
+
+
+def test_dataeditor_reactive_columns_js_semantics():


This test runs a copy of the JS that is written in the test. It does not run the expression that create() generates. If the generated expression changes or breaks, this test still passes.

Please evaluate the generated expression instead. For example, take str(...) of the columns prop for Var(_js_expr="state.columns"), and run it in Node with a stub state object. Then assert the default type, the default id, the explicit override, and the dict-row lookup.

FarhanAliRaza · 2026-10-19T08:05:02Z

@@ -0,0 +1 @@
+Apply the documented default column `type` and `id` to reactive (state-backed) DataEditor columns, fixing a render crash when a column omits them.


The file name uses the issue number. towncrier's issue_format turns 7596 into a link to the issue, not to this PR. Please rename the file to 7597.bugfix.md or +<slug>.bugfix.md.

…ced by the prop and cell callback

…rid prop and cell callback

anishgurjar · 2026-10-19T09:06:43Z

Addressed all six, PTAL

masenf

i like the idea here, but the implementation is taking this component away from the direction we're heading on the roadmap.

i have an alternate implementation that I will propose that leverages hooks in VarData worked out during create, so this component doesn't need to relying on post-create mutability to work

anishgurjar added 3 commits October 18, 2026 17:51

fix(dataeditor): apply default column type and id to reactive columns

3c10bf8

test(dataeditor): cover reactive columns default type and id

ca37b29

docs(dataeditor): news fragment for reflex-dev#7596

f8e31e7

anishgurjar requested a review from a team as a code owner October 18, 2026 21:52

cubic-dev-ai Bot reviewed Oct 18, 2026

View reviewed changes

Comment thread tests/units/components/datadisplay/test_dataeditor.py Outdated

Comment thread packages/reflex-components-dataeditor/src/reflex_components_dataeditor/dataeditor.py Outdated

greptile-apps Bot reviewed Oct 18, 2026

View reviewed changes

FarhanAliRaza added the on deck PRs lined up to review / merge next label Oct 19, 2026

FarhanAliRaza reviewed Oct 19, 2026

View reviewed changes

FarhanAliRaza requested changes Oct 19, 2026

View reviewed changes

anishgurjar added 3 commits October 19, 2026 05:04

fix(dataeditor): hoist reactive columns map into a hook const referen…

1febadb

…ced by the prop and cell callback

test(dataeditor): assert the hoisted columns const is shared by the g…

98e0f3b

…rid prop and cell callback

Rename 7596.bugfix.md to 7597.bugfix.md

4d1f07f

chore: refresh DataEditor generated stub hash

495ec4f

masenf added the bug Something isn't working label Oct 19, 2026 — with ChatGPT Codex Connector

masenf reviewed Oct 19, 2026

View reviewed changes

masenf removed the on deck PRs lined up to review / merge next label Oct 19, 2026

		assert 'id: (col.title ?? "").toLowerCase()' in _columns_prop(editor)


		def test_dataeditor_reactive_columns_js_semantics():

		@@ -0,0 +1 @@
		Apply the documented default column `type` and `id` to reactive (state-backed) DataEditor columns, fixing a render crash when a column omits them.

Conversation

anishgurjar commented Oct 18, 2026 • edited by cubic-dev-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Type of change

Testing

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 5/5

Findings

Summary

Uh oh!

Uh oh!

greptile-apps Bot Oct 18, 2026

Choose a reason for hiding this comment

Uh oh!

codspeed Bot commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Uh oh!

FarhanAliRaza left a comment

Choose a reason for hiding this comment

Uh oh!

FarhanAliRaza Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

FarhanAliRaza Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

FarhanAliRaza Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

FarhanAliRaza Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

FarhanAliRaza Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

FarhanAliRaza Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

anishgurjar commented Oct 19, 2026

Uh oh!

masenf left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

anishgurjar commented Oct 18, 2026 •

edited by cubic-dev-ai Bot

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

greptile-apps Bot commented Oct 18, 2026 •

edited

Loading

codspeed Bot commented Oct 19, 2026 •

edited

Loading