Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Prevent `reload_config()` from raising a duplicate-state error for state modules imported by `rxconfig.py`.
38 changes: 24 additions & 14 deletions packages/reflex-base/src/reflex_base/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -803,10 +803,12 @@ def _set_persistent(self, **kwargs):
self._replace_defaults(**kwargs)


# Project-local modules first imported while loading rxconfig.py; evicted
# before the next load so projects don't reuse each other's dependencies.
# Only mutated under _load_config_lock.
# Project-local modules first imported while loading rxconfig.py and the root
# that supplied them. They are evicted before loading a different project so
# projects don't reuse each other's dependencies. Only mutated under
# _load_config_lock.
_config_module_deps: set[str] = set()
_config_module_deps_root: Path | None = None


class _ImportRecorder:
Expand Down Expand Up @@ -894,7 +896,9 @@ def get_state_auto_setters() -> bool:
return False


def _get_config(project_root: Path | None = None) -> Config:
def _get_config(
project_root: Path | None = None, *, reload_dependencies: bool = True
) -> Config:
"""Import rxconfig.py fresh from the project root and return its config.

The project root is prepended to sys.path for the duration of the import so
Expand All @@ -907,10 +911,15 @@ def _get_config(project_root: Path | None = None) -> Config:
current working directory, resolved once up front so an rxconfig.py
that changes the cwd cannot move the root that the sys.path entry
and the dependency classification below are based on.
reload_dependencies: Whether to reload project-local modules imported by
rxconfig.py. A config reload in an existing RegistrationContext
keeps them so state classes are not redefined.

Returns:
The app config.
"""
global _config_module_deps_root

project_root = (project_root or Path.cwd()).resolve()
with _load_config_lock:
# A fresh str object, so the exact inserted entry can be removed by
Expand All @@ -919,16 +928,17 @@ def _get_config(project_root: Path | None = None) -> Config:
cwd = str(project_root)
sys.path.insert(0, cwd)
try:
# Never cache rxconfig or its project-local dependencies — each load
# goes to disk so different RegistrationContexts hold independent
# Config instances resolved against the current project. Evict
# before probing: find_spec answers from sys.modules, so modules
# left behind by another project directory would fake the existence
# check below.
# Always reload rxconfig, but retain its dependencies when reloading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Too long comments, explaining what is already can be seen in code. happens in many places.
we might want to clean up these.

# the same project. Re-importing a helper that defines State would
# redefine the class in the active RegistrationContext. Before
# switching projects, evict dependencies so find_spec and imports
# cannot reuse modules from the prior root.
sys.modules.pop(constants.Config.MODULE, None)
for dep in _config_module_deps:
sys.modules.pop(dep, None)
_config_module_deps.clear()
if reload_dependencies or _config_module_deps_root != project_root:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The _config_module_deps_root != project_root branch is not exercised. With the comparison removed, all 118 tests in tests/units/test_config.py still pass. test_get_config_evicts_dependencies_from_another_project calls _get_config() with the default reload_dependencies=True, so it never reaches this check.

Either drop _config_module_deps_root or add a test that reloads in one context after the cwd moved to a second project. Note that in that scenario a same-named state module would still hit the shadow error, so the guard may not buy anything.

for dep in _config_module_deps:
sys.modules.pop(dep, None)
_config_module_deps.clear()
_config_module_deps_root = project_root
# only import the module if it exists. If a module spec exists then
# the module exists.
if not find_spec(constants.Config.MODULE):
Expand Down Expand Up @@ -1021,6 +1031,6 @@ def reload_config() -> Config:
The freshly loaded app config.
"""
ctx = RegistrationContext.ensure_context()
config = _get_config()
config = _get_config(reload_dependencies=ctx._config is None)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1: When an older RegistrationContext is re-entered after another same-project context has loaded, ctx._config is None is false, so _get_config retains the globally cached dependency modules from the newer context. rxconfig.py then references state classes registered in the newer context while the active context retains its original classes, causing registry/runtime state mismatches; track dependency modules per context or otherwise restore the context-owned modules instead of using the global root-only cache.

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/config.py, line 1034:

<comment>When an older `RegistrationContext` is re-entered after another same-project context has loaded, `ctx._config is None` is false, so `_get_config` retains the globally cached dependency modules from the newer context. `rxconfig.py` then references state classes registered in the newer context while the active context retains its original classes, causing registry/runtime state mismatches; track dependency modules per context or otherwise restore the context-owned modules instead of using the global root-only cache.</comment>

<file context>
@@ -1021,6 +1031,6 @@ def reload_config() -> Config:
     """
     ctx = RegistrationContext.ensure_context()
-    config = _get_config()
+    config = _get_config(reload_dependencies=ctx._config is None)
     ctx._set_config(config)
     return config
</file context>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ctx._config is None is the wrong signal. RegistrationContext.fork() copies base_states but resets _config to None. So a forked context evicts and re-imports the state module into a context that already holds the class, and the shadow check fires again.

Repro with the appmod.py / rxconfig.py pair from #7028:

with RegistrationContext() as ctx:
    c.get_config()
    forked = ctx.fork()
    tok = RegistrationContext.set(forked)
    c.reload_config()
    # StateValueError: The substate class 'appmod____my_state' has been defined multiple times.

This is the path AppHarness takes (reflex/testing.py:286: fork, then reload_config()), so the harness still crashes on such a project. Please key the decision on whether the current context already holds the states those modules registered, not on whether it has a cached config. Add the fork case to the tests.

ctx._set_config(config)
return config
100 changes: 99 additions & 1 deletion tests/units/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -1029,20 +1029,118 @@ def test_get_config_accepts_explicit_project_root(
assert reflex_base.config._get_config(project).app_name == "explicit"


def test_reload_config_does_not_redefine_project_state(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch, clean_config_modules: None
):
"""Reloading config does not re-import project modules that define state.

Args:
tmp_path: The pytest tmp_path fixture.
monkeypatch: The pytest monkeypatch fixture.
clean_config_modules: Cleanup for modules left behind by the load.
"""
from reflex_base.registry import RegistrationContext

(tmp_path / "config_reload_state_module.py").write_text(
"import reflex as rx\n\nclass MyState(rx.State):\n value: str = ''\n"
)
(tmp_path / "rxconfig.py").write_text(
"import config_reload_state_module\nimport reflex as rx\n\n"
"config = rx.Config(app_name='state_reload')\n"
)
monkeypatch.chdir(tmp_path)

with RegistrationContext():
assert reflex_base.config.get_config().app_name == "state_reload"
assert reflex_base.config.reload_config().app_name == "state_reload"


def test_get_config_reloads_project_state_for_each_context(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch, clean_config_modules: None
):
"""Initial config loads register project state in each context.

Args:
tmp_path: The pytest tmp_path fixture.
monkeypatch: The pytest monkeypatch fixture.
clean_config_modules: Cleanup for modules left behind by the load.
"""
from reflex_base.registry import RegistrationContext

(tmp_path / "config_reload_state_module.py").write_text(
"import reflex as rx\n\nclass MyState(rx.State):\n value: str = ''\n"
)
(tmp_path / "rxconfig.py").write_text(
"import config_reload_state_module\nimport reflex as rx\n\n"
"config = rx.Config(app_name='state_reload')\n"
)
monkeypatch.chdir(tmp_path)

with RegistrationContext() as first_context:
reflex_base.config.get_config()
first_state = next(
state
for state in first_context.base_states.values()
if state.__module__ == "config_reload_state_module"
)

with RegistrationContext() as second_context:
reflex_base.config.get_config()
second_state = next(
state
for state in second_context.base_states.values()
if state.__module__ == "config_reload_state_module"
)

assert second_state is not first_state


def test_get_config_evicts_dependencies_from_another_project(
tmp_path: Path, clean_config_modules: None
):
"""Loading another project does not reuse a same-named local dependency.

Args:
tmp_path: The pytest tmp_path fixture.
clean_config_modules: Cleanup for modules left behind by the load.
"""
first_project = tmp_path / "first"
second_project = tmp_path / "second"
for project, app_name in ((first_project, "first"), (second_project, "second")):
project.mkdir()
(project / "config_reload_dependency.py").write_text(
f"APP_NAME = {app_name!r}\n"
)
(project / "rxconfig.py").write_text(
"import config_reload_dependency\nimport reflex as rx\n\n"
"config = rx.Config(app_name=config_reload_dependency.APP_NAME)\n"
)

assert reflex_base.config._get_config(first_project).app_name == "first"
assert reflex_base.config._get_config(second_project).app_name == "second"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Both calls use reload_dependencies=True, so this passes on main too (after the fixture is adjusted) and does not cover the new root check. To cover it, load first_project into a context, then call reload_config() from second_project in the same context.



@pytest.fixture
def clean_config_modules() -> Generator[None, None, None]:
"""Drop the modules and dep records a real rxconfig load leaves behind.

Yields:
None, once the module table is clean.
"""
names = ("rxconfig", "side_module", "chdir_dep_module")
names = (
"rxconfig",
"side_module",
"chdir_dep_module",
"config_reload_state_module",
"config_reload_dependency",
)
try:
yield
finally:
for name in names:
sys.modules.pop(name, None)
reflex_base.config._config_module_deps.clear()
reflex_base.config._config_module_deps_root = None


# Reruns: taking the prepended entry back out is itself a sys.path shrink, so
Expand Down
Loading