Skip to content

Commit e8b5724

Browse files
fix: use actual parent class for sibling grouping in minify sync/validate
sync_minify_config and validate_minify_config grouped siblings by string-splitting state paths, which fails when children of the same parent class are defined in different Python modules. This caused colliding minified IDs (e.g. both getting 'a'). Now uses get_parent_state() class objects for grouping, consistent with generate_minify_config which was already correct. Also resolves merge conflicts in _get_event_handler to keep the minified event name reverse lookup.
1 parent 9bd6ad1 commit e8b5724

4 files changed

Lines changed: 108 additions & 50 deletions

File tree

reflex/minify.py

Lines changed: 33 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -400,25 +400,24 @@ def validate_minify_config(
400400

401401
all_states = collect_all_states(root_state)
402402

403-
# Check for duplicate state IDs among siblings
404-
# Group states by parent path and check for duplicate minified names
405-
parent_to_state_ids: dict[str | None, dict[str, list[str]]] = {}
403+
# Check for duplicate state IDs among siblings.
404+
# Group by actual parent class (not string-split path) since children of
405+
# the same parent can be defined in different modules.
406+
path_to_cls = {get_state_full_path(s): s for s in all_states}
407+
parent_cls_to_state_ids: dict[type[BaseState] | None, dict[str, list[str]]] = {}
406408
for state_path, minified_name in config["states"].items():
407-
# Get parent path
408-
parts = state_path.rsplit(".", 1)
409-
parent_path = parts[0] if len(parts) > 1 else None
409+
state_cls = path_to_cls.get(state_path)
410+
parent_cls = state_cls.get_parent_state() if state_cls else None
411+
parent_cls_to_state_ids.setdefault(parent_cls, {}).setdefault(
412+
minified_name, []
413+
).append(state_path)
410414

411-
if parent_path not in parent_to_state_ids:
412-
parent_to_state_ids[parent_path] = {}
413-
if minified_name not in parent_to_state_ids[parent_path]:
414-
parent_to_state_ids[parent_path][minified_name] = []
415-
parent_to_state_ids[parent_path][minified_name].append(state_path)
416-
417-
for parent_path, id_to_states in parent_to_state_ids.items():
415+
for parent_cls, id_to_states in parent_cls_to_state_ids.items():
418416
for minified_name, state_paths in id_to_states.items():
419417
if len(state_paths) > 1:
418+
parent_name = parent_cls.__name__ if parent_cls else "root"
420419
errors.append(
421-
f"Duplicate state_id='{minified_name}' under '{parent_path or 'root'}': "
420+
f"Duplicate state_id='{minified_name}' under '{parent_name}': "
422421
f"{state_paths}"
423422
)
424423

@@ -525,31 +524,30 @@ def sync_minify_config(
525524
# Remove empty event dicts
526525
new_events = {k: v for k, v in new_events.items() if v}
527526

528-
# Find states that need IDs assigned
529-
# Group by parent for sibling-unique assignment
530-
parent_to_children: dict[str | None, list[str]] = {}
527+
# Build a map from actual parent class to existing sibling minified IDs.
528+
# Using the live state classes avoids the string-split bug where children
529+
# of the same parent class defined in different modules get different
530+
# string-based parent paths and are assigned colliding IDs.
531+
parent_cls_to_existing_ids: dict[type[BaseState] | None, set[int]] = {}
532+
for state_cls in all_states:
533+
state_path = get_state_full_path(state_cls)
534+
if state_path in new_states:
535+
parent = state_cls.get_parent_state()
536+
parent_cls_to_existing_ids.setdefault(parent, set()).add(
537+
minified_name_to_int(new_states[state_path])
538+
)
539+
540+
# Find states that need IDs assigned, grouped by actual parent class.
541+
parent_cls_to_new_children: dict[type[BaseState] | None, list[str]] = {}
531542
for state_cls in all_states:
532543
state_path = get_state_full_path(state_cls)
533544
if state_path not in new_states:
534545
parent = state_cls.get_parent_state()
535-
parent_path = get_state_full_path(parent) if parent else None
536-
if parent_path not in parent_to_children:
537-
parent_to_children[parent_path] = []
538-
parent_to_children[parent_path].append(state_path)
539-
540-
# Assign new state IDs
541-
for parent_path, children in parent_to_children.items():
542-
# Get existing IDs for this parent's children (convert to ints for finding max)
543-
existing_ids: set[int] = set()
544-
for state_path, minified_name in new_states.items():
545-
parts = state_path.rsplit(".", 1)
546-
sp_parent = parts[0] if len(parts) > 1 else None
547-
# Compare parent paths correctly
548-
if parent_path is None:
549-
if sp_parent is None or "." not in state_path:
550-
existing_ids.add(minified_name_to_int(minified_name))
551-
elif sp_parent == parent_path:
552-
existing_ids.add(minified_name_to_int(minified_name))
546+
parent_cls_to_new_children.setdefault(parent, []).append(state_path)
547+
548+
# Assign new state IDs (unique among siblings of the same parent class)
549+
for parent_cls, children in parent_cls_to_new_children.items():
550+
existing_ids = parent_cls_to_existing_ids.get(parent_cls, set()).copy()
553551

554552
# Assign IDs starting from max + 1 (or 0 if reassign_deleted and gaps exist)
555553
next_id = 0 if reassign_deleted else (max(existing_ids, default=-1) + 1)

reflex/state.py

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1780,7 +1780,6 @@ async def get_var_value(self, var: Var[VAR_TYPE]) -> VAR_TYPE:
17801780
)
17811781
return getattr(other_state, var_data.field_name)
17821782

1783-
<<<<<<< .merge_file_rqeWiK
17841783
@classmethod
17851784
def _get_original_event_name(cls, minified_name: str) -> str | None:
17861785
"""Look up the original event handler name from a minified name.
@@ -1797,8 +1796,6 @@ def _get_original_event_name(cls, minified_name: str) -> str | None:
17971796
# Direct lookup: _event_id_to_name maps minified_name -> original_name
17981797
return cls._event_id_to_name.get(minified_name)
17991798

1800-
=======
1801-
>>>>>>> .merge_file_20JAsy
18021799
def _get_event_handler(self, event: Event | str) -> tuple[BaseState, EventHandler]:
18031800
"""Get the event handler for the given event.
18041801
@@ -1821,7 +1818,6 @@ def _get_event_handler(self, event: Event | str) -> tuple[BaseState, EventHandle
18211818
msg = "The value of state cannot be None when processing an event."
18221819
raise ValueError(msg)
18231820

1824-
<<<<<<< .merge_file_rqeWiK
18251821
# Try to look up the handler directly first
18261822
handler = substate.event_handlers.get(name)
18271823
if handler is None:
@@ -1833,8 +1829,6 @@ def _get_event_handler(self, event: Event | str) -> tuple[BaseState, EventHandle
18331829
msg = f"Event handler '{name}' not found in state '{type(substate).__name__}'"
18341830
raise KeyError(msg)
18351831

1836-
=======
1837-
>>>>>>> .merge_file_20JAsy
18381832
return substate, handler
18391833

18401834
async def _process(self, event: Event) -> AsyncIterator[StateUpdate]:

tests/units/istate/test_proxy.py

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import dataclasses
44
import pickle
5-
<<<<<<< .merge_file_7wIbhS
65
from asyncio import CancelledError
76
from contextlib import asynccontextmanager
87
from unittest.mock import patch
@@ -11,11 +10,6 @@
1110

1211
import reflex as rx
1312
from reflex.istate.proxy import MutableProxy, StateProxy
14-
=======
15-
16-
import reflex as rx
17-
from reflex.istate.proxy import MutableProxy
18-
>>>>>>> .merge_file_giC46q
1913

2014

2115
@dataclasses.dataclass
@@ -46,7 +40,6 @@ def test_mutable_proxy_pickle_preserves_object_identity():
4640
assert unpickled["direct"][0].id == 1
4741
assert unpickled["proxied"][0].id == 1
4842
assert unpickled["direct"][0] is unpickled["proxied"][0]
49-
<<<<<<< .merge_file_7wIbhS
5043

5144

5245
@pytest.mark.usefixtures("mock_app")
@@ -73,5 +66,3 @@ async def mock_modify_state_context(*args, **kwargs): # noqa: RUF029
7366
# After the exception, we should be able to enter the context again without issues
7467
async with state_proxy:
7568
pass
76-
=======
77-
>>>>>>> .merge_file_giC46q

tests/units/test_minification.py

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,81 @@ def handler_b(self):
318318
new_config["events"][state_path]["handler_b"] == "l"
319319
) # 10 + 1 = 11 -> 'l'
320320

321+
def test_sync_no_sibling_collision_across_modules(self):
322+
"""Test that sync assigns unique IDs to siblings of the same parent.
323+
324+
When children of the same parent state class are defined in different
325+
Python modules, their get_state_full_path() produces different string
326+
prefixes. The sync function must group siblings by the actual parent
327+
class object, not by string-splitting the path, to avoid ID collisions.
328+
"""
329+
330+
class ParentState(BaseState):
331+
pass
332+
333+
class ChildA(ParentState):
334+
pass
335+
336+
class ChildB(ParentState):
337+
pass
338+
339+
parent_path = get_state_full_path(ParentState)
340+
child_a_path = get_state_full_path(ChildA)
341+
342+
# Config already has ParentState and ChildA assigned
343+
existing_config: MinifyConfig = {
344+
"version": SCHEMA_VERSION,
345+
"states": {
346+
parent_path: "a",
347+
child_a_path: "a",
348+
},
349+
"events": {},
350+
}
351+
352+
# Sync should assign ChildB a DIFFERENT ID than ChildA
353+
new_config = sync_minify_config(existing_config, ParentState)
354+
355+
child_b_path = get_state_full_path(ChildB)
356+
assert child_b_path in new_config["states"]
357+
assert (
358+
new_config["states"][child_a_path] != new_config["states"][child_b_path]
359+
), (
360+
f"Sibling collision: ChildA and ChildB both got "
361+
f"'{new_config['states'][child_a_path]}'"
362+
)
363+
364+
def test_validate_detects_sibling_collision(self):
365+
"""Test that validate catches duplicate IDs among siblings of same parent."""
366+
367+
class ParentState(BaseState):
368+
pass
369+
370+
class ChildA(ParentState):
371+
pass
372+
373+
class ChildB(ParentState):
374+
pass
375+
376+
parent_path = get_state_full_path(ParentState)
377+
child_a_path = get_state_full_path(ChildA)
378+
child_b_path = get_state_full_path(ChildB)
379+
380+
# Manually create a config with colliding sibling IDs
381+
bad_config: MinifyConfig = {
382+
"version": SCHEMA_VERSION,
383+
"states": {
384+
parent_path: "a",
385+
child_a_path: "a",
386+
child_b_path: "a", # collision!
387+
},
388+
"events": {},
389+
}
390+
391+
errors, _warnings, _missing = validate_minify_config(bad_config, ParentState)
392+
assert any("Duplicate" in e and "'a'" in e for e in errors), (
393+
f"Expected duplicate ID error, got: {errors}"
394+
)
395+
321396

322397
class TestStateMinification:
323398
"""Tests for state name minification with minify.json."""

0 commit comments

Comments
 (0)