diff --git a/reflex/app.py b/reflex/app.py index f2226f5d6ee..79073fe08c8 100644 --- a/reflex/app.py +++ b/reflex/app.py @@ -35,7 +35,11 @@ from reflex.app_mixins import AppMixin, LifespanMixin, MiddlewareMixin from reflex.compiler import compiler from reflex.compiler import utils as compiler_utils -from reflex.compiler.compiler import ExecutorSafeFunctions, compile_theme +from reflex.compiler.compiler import ( + ExecutorSafeFunctions, + compile_theme, + readable_name_from_component, +) from reflex.components.base.app_wrap import AppWrap from reflex.components.base.error_boundary import ErrorBoundary from reflex.components.base.fragment import Fragment @@ -284,6 +288,25 @@ class UnevaluatedPage: meta: list[dict[str, str]] context: dict[str, Any] | None + def merged_with(self, other: UnevaluatedPage) -> UnevaluatedPage: + """Merge the other page into this one. + + Args: + other: The other page to merge with. + + Returns: + The merged page. + """ + return dataclasses.replace( + self, + title=self.title if self.title is not None else other.title, + description=self.description + if self.description is not None + else other.description, + on_load=self.on_load if self.on_load is not None else other.on_load, + context=self.context if self.context is not None else other.context, + ) + @dataclasses.dataclass() class App(MiddlewareMixin, LifespanMixin): @@ -719,22 +742,37 @@ def add_page( # Check if the route given is valid verify_route_validity(route) - if route in self._unevaluated_pages and environment.RELOAD_CONFIG.is_set(): - # when the app is reloaded(typically for app harness tests), we should maintain - # the latest render function of a route.This applies typically to decorated pages - # since they are only added when app._compile is called. - self._unevaluated_pages.pop(route) + unevaluated_page = UnevaluatedPage( + component=component, + route=route, + title=title, + description=description, + image=image, + on_load=on_load, + meta=meta, + context=context, + ) if route in self._unevaluated_pages: - route_name = ( - f"`{route}` or `/`" - if route == constants.PageNames.INDEX_ROUTE - else f"`{route}`" - ) - raise exceptions.RouteValueError( - f"Duplicate page route {route_name} already exists. Make sure you do not have two" - f" pages with the same route" - ) + if self._unevaluated_pages[route].component is component: + unevaluated_page = unevaluated_page.merged_with( + self._unevaluated_pages[route] + ) + console.warn( + f"Page {route} is being redefined with the same component." + ) + else: + route_name = ( + f"`{route}` or `/`" + if route == constants.PageNames.INDEX_ROUTE + else f"`{route}`" + ) + existing_component = self._unevaluated_pages[route].component + raise exceptions.RouteValueError( + f"Tried to add page {readable_name_from_component(component)} with route {route_name} but " + f"page {readable_name_from_component(existing_component)} with the same route already exists. " + "Make sure you do not have two pages with the same route." + ) # Setup dynamic args for the route. # this state assignment is only required for tests using the deprecated state kwarg for App @@ -746,16 +784,7 @@ def add_page( on_load if isinstance(on_load, list) else [on_load] ) - self._unevaluated_pages[route] = UnevaluatedPage( - component=component, - route=route, - title=title, - description=description, - image=image, - on_load=on_load, - meta=meta, - context=context, - ) + self._unevaluated_pages[route] = unevaluated_page def _compile_page(self, route: str, save_page: bool = True): """Compile a page. @@ -1021,9 +1050,11 @@ def _apply_decorated_pages(self): This can move back into `compile_` when py39 support is dropped. """ + app_name = get_config().app_name # Add the @rx.page decorated pages to collect on_load events. - for render, kwargs in DECORATED_PAGES[get_config().app_name]: + for render, kwargs in DECORATED_PAGES[app_name]: self.add_page(render, **kwargs) + DECORATED_PAGES[app_name].clear() def _validate_var_dependencies(self, state: type[BaseState] | None = None) -> None: """Validate the dependencies of the vars in the app. diff --git a/reflex/compiler/compiler.py b/reflex/compiler/compiler.py index 0b309d008f8..4d26e691db2 100644 --- a/reflex/compiler/compiler.py +++ b/reflex/compiler/compiler.py @@ -4,6 +4,7 @@ from collections.abc import Iterable, Sequence from datetime import datetime +from inspect import getmodule from pathlib import Path from typing import TYPE_CHECKING @@ -676,6 +677,35 @@ def _into_component_once( return None +def readable_name_from_component( + component: Component | ComponentCallable, +) -> str | None: + """Get the readable name of a component. + + Args: + component: The component to get the name of. + + Returns: + The readable name of the component. + """ + if isinstance(component, Component): + return type(component).__name__ + if isinstance(component, (Var, int, float, str)): + return str(component) + if isinstance(component, Sequence): + return ", ".join(str(c) for c in component) + if callable(component): + module_name = getattr(component, "__module__", None) + if module_name is not None: + module = getmodule(component) + if module is not None: + module_name = module.__name__ + if module_name is not None: + return f"{module_name}.{component.__name__}" + return component.__name__ + return None + + def into_component(component: Component | ComponentCallable) -> Component: """Convert a component to a Component. diff --git a/reflex/config.py b/reflex/config.py index 8bbb3605e62..4ac5d5b6397 100644 --- a/reflex/config.py +++ b/reflex/config.py @@ -695,9 +695,6 @@ class EnvironmentVariables: # The port to run the backend on. REFLEX_BACKEND_PORT: EnvVar[int | None] = env_var(None) - # Reflex internal env to reload the config. - RELOAD_CONFIG: EnvVar[bool] = env_var(False, internal=True) - # If this env var is set to "yes", App.compile will be a no-op REFLEX_SKIP_COMPILE: EnvVar[bool] = env_var(False, internal=True) diff --git a/reflex/testing.py b/reflex/testing.py index 1616f77e734..6cae0d4629a 100644 --- a/reflex/testing.py +++ b/reflex/testing.py @@ -116,7 +116,6 @@ class AppHarness: backend: uvicorn.Server | None = None state_manager: StateManager | None = None _frontends: list[WebDriver] = dataclasses.field(default_factory=list) - _decorated_pages: list = dataclasses.field(default_factory=list) @classmethod def create( @@ -267,8 +266,6 @@ def _initialize_app(self): with chdir(self.app_path): # ensure config and app are reloaded when testing different app reflex.config.get_config(reload=True) - # Save decorated pages before importing the test app module - before_decorated_pages = reflex.app.DECORATED_PAGES[self.app_name].copy() # Ensure the AppHarness test does not skip State assignment due to running via pytest os.environ.pop(reflex.constants.PYTEST_CURRENT_TEST, None) os.environ[reflex.constants.APP_HARNESS_FLAG] = "true" @@ -276,12 +273,6 @@ def _initialize_app(self): # Do not reload the module for pre-existing apps (only apps generated from source) reload=self.app_source is not None ) - # Save the pages that were added during testing - self._decorated_pages = [ - p - for p in reflex.app.DECORATED_PAGES[self.app_name] - if p not in before_decorated_pages - ] self.app_instance = self.app_module.app if self.app_instance and isinstance( self.app_instance._state_manager, StateManagerRedis @@ -500,10 +491,6 @@ def stop(self) -> None: if self.frontend_output_thread is not None: self.frontend_output_thread.join() - # Cleanup decorated pages added during testing - for page in self._decorated_pages: - reflex.app.DECORATED_PAGES[self.app_name].remove(page) - def __exit__(self, *excinfo) -> None: """Contextmanager protocol for `stop()`. diff --git a/reflex/utils/prerequisites.py b/reflex/utils/prerequisites.py index a4612d5b5ce..6e4f341b7d1 100644 --- a/reflex/utils/prerequisites.py +++ b/reflex/utils/prerequisites.py @@ -371,7 +371,6 @@ def get_app(reload: bool = False) -> ModuleType: from reflex.utils import telemetry try: - environment.RELOAD_CONFIG.set(reload) config = get_config() _check_app_name(config) @@ -384,11 +383,14 @@ def get_app(reload: bool = False) -> ModuleType: else config.app_module ) if reload: + from reflex.page import DECORATED_PAGES from reflex.state import reload_state_module # Reset rx.State subclasses to avoid conflict when reloading. reload_state_module(module=module) + DECORATED_PAGES.clear() + # Reload the app module. importlib.reload(app) except Exception as ex: diff --git a/tests/units/test_app.py b/tests/units/test_app.py index 9adfbef7596..f252cd4dfe3 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -15,6 +15,7 @@ import pytest import sqlmodel from fastapi import FastAPI, UploadFile +from pytest_mock import MockerFixture from starlette_admin.auth import AuthProvider from starlette_admin.contrib.sqla.admin import Admin from starlette_admin.contrib.sqla.view import ModelView @@ -49,7 +50,7 @@ _substate_key, ) from reflex.style import Style -from reflex.utils import exceptions, format +from reflex.utils import console, exceptions, format from reflex.vars.base import computed_var from .conftest import chdir @@ -332,7 +333,28 @@ def index(): @pytest.mark.parametrize( - "first_page,second_page, route", + ("first_page", "second_page", "route"), + [ + (index, index, None), + (page1, page1, None), + ], +) +def test_add_the_same_page( + mocker: MockerFixture, app: App, first_page, second_page, route +): + app.add_page(first_page, route=route) + mock_object = mocker.Mock() + mocker.patch.object( + console, + "warn", + mock_object, + ) + app.add_page(second_page, route="/" + route.strip("/") if route else None) + assert mock_object.call_count == 1 + + +@pytest.mark.parametrize( + ("first_page", "second_page", "route"), [ (lambda: rx.fragment(), lambda: rx.fragment(rx.text("second")), "/"), (rx.fragment(rx.text("first")), rx.fragment(rx.text("second")), "/page1"), @@ -342,11 +364,9 @@ def index(): "page3", ), (page1, page2, "page1"), - (index, index, None), - (page1, page1, None), ], ) -def test_add_duplicate_page_route_error(app, first_page, second_page, route): +def test_add_duplicate_page_route_error(app: App, first_page, second_page, route): app.add_page(first_page, route=route) with pytest.raises(ValueError): app.add_page(second_page, route="/" + route.strip("/") if route else None)