diff --git a/news/6457.feature.md b/news/6457.feature.md new file mode 100644 index 00000000000..f7c5e1d89ff --- /dev/null +++ b/news/6457.feature.md @@ -0,0 +1 @@ +Auto-memoized (`rx.memo`) components now compile to `.web` output paths that mirror their defining Python source module instead of being bundled into a single shared `components.jsx`. The compiler's auto-memo registry is scoped per source module, so identical-rendering subtrees in different modules each emit their own output instead of one silently overwriting another, hot-reloads of a module refresh the correct output, and stale memo files are cleaned up when their source changes. diff --git a/packages/reflex-base/news/6457.feature.md b/packages/reflex-base/news/6457.feature.md new file mode 100644 index 00000000000..baa7acf952f --- /dev/null +++ b/packages/reflex-base/news/6457.feature.md @@ -0,0 +1 @@ +Added `reflex_base.utils.memo_paths`, which translates a memo's Python source module into the mirrored `.web` JSX path and `$/...` library specifier used by the compiler. The memo component and compiler plugin now route each memo's compiled output through these helpers so it lands alongside its source module's layout. diff --git a/packages/reflex-base/src/reflex_base/compiler/templates.py b/packages/reflex-base/src/reflex_base/compiler/templates.py index 0d2f750e9eb..a380aaac4b9 100644 --- a/packages/reflex-base/src/reflex_base/compiler/templates.py +++ b/packages/reflex-base/src/reflex_base/compiler/templates.py @@ -8,6 +8,7 @@ from reflex_base import constants from reflex_base.constants import Hooks +from reflex_base.utils import memo_paths from reflex_base.utils.format import format_state_name, json_dumps from reflex_base.vars.base import VarData @@ -192,7 +193,7 @@ def app_root_template( if hydrate_fallback_export is not None: hydrate_fallback_str = ( f"export {{ {hydrate_fallback_export} as HydrateFallback }} " - f'from "$/{constants.Dirs.COMPONENTS_PATH}/{hydrate_fallback_export}";' + f'from "{memo_paths.unmirrored_library_specifier(hydrate_fallback_export)}";' ) custom_code_str = "\n".join(custom_codes) diff --git a/packages/reflex-base/src/reflex_base/components/memo.py b/packages/reflex-base/src/reflex_base/components/memo.py index c31e37dd9db..e39f5b77e80 100644 --- a/packages/reflex-base/src/reflex_base/components/memo.py +++ b/packages/reflex-base/src/reflex_base/components/memo.py @@ -39,7 +39,7 @@ ) from reflex_base.constants.state import CAMEL_CASE_MEMO_MARKER from reflex_base.event import EventChain, EventHandler, no_args_event_spec, run_script -from reflex_base.utils import console, format +from reflex_base.utils import console, format, memo_paths from reflex_base.utils.imports import ImportVar from reflex_base.utils.types import safe_issubclass, typehint_issubclass from reflex_base.vars import VarData @@ -253,6 +253,12 @@ class MemoDefinition: fn: Callable[..., Any] python_name: str params: tuple[MemoParam, ...] + # The user-app Python module that defined this memo. When set, the memo's + # compiled JSX is emitted to a path mirroring that module and the page-side + # import resolves there instead of the per-name + # ``app_components/_internal/`` path used for memos that can't be + # mirrored. ``kw_only`` so subclasses can keep their own required fields. + source_module: str | None = dataclasses.field(default=None, kw_only=True) @dataclasses.dataclass(frozen=True, slots=True) @@ -300,7 +306,7 @@ def component(self) -> Component: class MemoComponent(Component): """A rendered instance of a memo component.""" - library = f"$/{constants.Dirs.COMPONENTS_PATH}" + library = f"$/{constants.Dirs.APP_COMPONENTS_INTERNAL}" _memoization_mode = MemoizationMode(disposition=MemoizationDisposition.NEVER) # The user-authored component class this wrapper stands in for. Populated @@ -346,6 +352,7 @@ def _post_init(self, **kwargs): def _get_memo_component_class( export_name: str, wrapped_component_type: type[Component] = Component, + source_module: str | None = None, ) -> type[MemoComponent]: """Get the component subclass for a memo export. @@ -360,18 +367,21 @@ def _get_memo_component_class( wrapped_component_type: The class of the component being memoized. Defaults to ``Component`` for memos that don't wrap a user component (e.g. function memos, raw passthroughs). + source_module: The user-app Python module that defined this memo. When + set, the wrapper imports from a path mirroring that module instead + of the per-name ``app_components/_internal/`` path. Returns: A cached component subclass with the tag set at class definition time. """ + # With a source module the memo is grouped into a file mirroring its + # Python module; otherwise each memo gets its own per-file module so Vite + # has distinct module boundaries per memo, enabling code-split by page. + library = memo_paths.library_for(source_module, export_name) attrs: dict[str, Any] = { "__module__": __name__, "tag": export_name, - # Point each memo at its own per-file module so pages import directly - # from ``$/utils/components/`` rather than through the index. - # Per-file import paths give Vite distinct module boundaries per - # memo, enabling actual code-split by page. - "library": f"$/{constants.Dirs.COMPONENTS_PATH}/{export_name}", + "library": library, "_wrapped_component_type": wrapped_component_type, } if ( @@ -388,6 +398,18 @@ def _get_memo_component_class( ) +def reset_memo_component_classes() -> None: + """Clear the cached memo wrapper classes. + + Called at the start of each compile so a memo's ``library`` is recomputed + from the current module layout. Without this, a module that switches to a + package (or back) between hot-reload compiles would keep serving the + library specifier resolved on the first compile, pointing pages at an + output path the compiler no longer writes. + """ + _get_memo_component_class.cache_clear() + + MEMOS: dict[str, MemoDefinition] = {} @@ -627,42 +649,48 @@ def _get_rest_param(params: tuple[MemoParam, ...]) -> MemoParam | None: return next((p for p in params if p.kind is MemoParamKind.REST), None) -def _imported_function_var(name: str, return_type: Any) -> FunctionVar: +def _imported_function_var( + name: str, return_type: Any, source_module: str | None = None +) -> FunctionVar: """Create the imported FunctionVar for a memo. Args: name: The exported function name. return_type: The return type of the function. + source_module: The user-app Python module that defined the memo. When + set, the import resolves to the mirrored module file instead of the + per-name ``app_components/_internal/`` path. Returns: The imported FunctionVar. """ + library = memo_paths.library_for(source_module, name) return FunctionStringVar.create( name, _var_type=ReflexCallable[Any, return_type], - _var_data=VarData( - imports={ - f"$/{constants.Dirs.COMPONENTS_PATH}/{name}": [ImportVar(tag=name)] - } - ), + _var_data=VarData(imports={library: [ImportVar(tag=name)]}), ) -def _component_import_var(name: str) -> Var: +def _component_import_var(name: str, source_module: str | None = None) -> Var: """Create the imported component var for a memo component. Args: name: The exported component name. + source_module: The user-app Python module that defined the memo. When + set, the import resolves to the mirrored module file instead of the + per-name ``app_components/_internal/`` path. Returns: The component var. """ + library = memo_paths.library_for(source_module, name) return Var( name, _var_type=type[Component], _var_data=VarData( imports={ - f"$/{constants.Dirs.COMPONENTS_PATH}/{name}": [ImportVar(tag=name)], + library: [ImportVar(tag=name)], "@emotion/react": [ImportVar(tag="jsx")], } ), @@ -1311,12 +1339,14 @@ def _evaluate_function_body( def _create_component_definition( fn: Callable[..., Any], return_annotation: Any, + source_module: str | None = None, ) -> MemoComponentDefinition: """Create a definition for a component-returning memo. Args: fn: The function to analyze. return_annotation: The return annotation. + source_module: The user-app Python module that defined the memo. Returns: The component memo definition. @@ -1329,6 +1359,7 @@ def _create_component_definition( fn=fn, python_name=fn.__name__, params=params, + source_module=source_module, export_name=format.to_title_case(fn.__name__), _component=_LazyBody.ready(_evaluate_component_body(fn, params)), ) @@ -1594,7 +1625,9 @@ def __call__(self, *children: Any, **props: Any) -> MemoComponent: # Reading ``component`` materializes the deferred body, so ``type(...)`` # reflects the real wrapped class rather than the placeholder. return _get_memo_component_class( - definition.export_name, type(definition.component) + definition.export_name, + type(definition.component), + definition.source_module, )._create( children=list(children), memo_definition=definition, @@ -1608,7 +1641,9 @@ def _as_var(self) -> Var: Returns: The imported component var. """ - return _component_import_var(self._definition.export_name) + return _component_import_var( + self._definition.export_name, self._definition.source_module + ) def _create_function_wrapper( @@ -1641,6 +1676,7 @@ def _create_component_wrapper( def create_passthrough_component_memo( component: Component, + source_module: str | None = None, ) -> tuple[ Callable[..., MemoComponent], MemoComponentDefinition, @@ -1659,6 +1695,8 @@ def create_passthrough_component_memo( Args: component: The component to wrap. + source_module: The user-app Python module that triggered creation of + this memo (typically the page that contained the wrapped subtree). Returns: The callable memo wrapper and its component definition. @@ -1726,7 +1764,7 @@ def passthrough(children: Var[Component]) -> Component: passthrough.__qualname__ = passthrough.__name__ passthrough.__module__ = __name__ - definition = _create_component_definition(passthrough, Component) + definition = _create_component_definition(passthrough, Component, source_module) replacements: dict[str, Any] = {} if definition.export_name != tag: replacements["export_name"] = tag @@ -1871,6 +1909,8 @@ def memo(fn: Callable[..., Any]) -> _MemoComponentWrapper | _MemoFunctionWrapper defaulted_params=defaulted_params, ) + source_module = memo_paths.capture_source_module(fn) + if missing_return or defaulted_params: _warn_missing_annotations(fn.__name__, missing_return, defaulted_params) @@ -1885,6 +1925,7 @@ def memo(fn: Callable[..., Any]) -> _MemoComponentWrapper | _MemoFunctionWrapper fn=fn, python_name=fn.__name__, params=params, + source_module=source_module, export_name=format.to_title_case(fn.__name__), _component=_LazyBody( lambda: _evaluate_component_body(fn, params), @@ -1897,9 +1938,12 @@ def memo(fn: Callable[..., Any]) -> _MemoComponentWrapper | _MemoFunctionWrapper fn=fn, python_name=fn.__name__, params=params, + source_module=source_module, _function=_LazyBody(lambda: _evaluate_function_body(fn, params)), imported_var=_imported_function_var( - fn.__name__, _annotation_inner_type(return_annotation) + fn.__name__, + _annotation_inner_type(return_annotation), + source_module=source_module, ), ) wrapper = _create_function_wrapper(definition) diff --git a/packages/reflex-base/src/reflex_base/constants/base.py b/packages/reflex-base/src/reflex_base/constants/base.py index 620d9bd7ee0..1fd74df3dd3 100644 --- a/packages/reflex-base/src/reflex_base/constants/base.py +++ b/packages/reflex-base/src/reflex_base/constants/base.py @@ -32,8 +32,6 @@ class Dirs(SimpleNamespace): UTILS = "utils" # The name of the state file. STATE_PATH = UTILS + "/state" - # The name of the components file. - COMPONENTS_PATH = UTILS + "/components" # The name of the contexts file. CONTEXTS_PATH = UTILS + "/context" # The name of the output directory. @@ -48,6 +46,14 @@ class Dirs(SimpleNamespace): PAGES = "app" # The name of the routes directory. ROUTES = "routes" + # Subdirectory holding memo modules mirrored from user Python modules, + # kept separate from framework output so user module paths can't collide. + APP_COMPONENTS = "app_components" + # Reserved subdir for memos that can't be mirrored to a user module + # (framework components, ``__main__``, unsafe names). The leading underscore + # keeps it clear of mirrored user-module trees, since top-level Python + # packages conventionally never start with ``_``. + APP_COMPONENTS_INTERNAL = APP_COMPONENTS + "/_internal" # The name of the env json file. ENV_JSON = "env.json" # The name of the reflex json file. diff --git a/packages/reflex-base/src/reflex_base/plugins/compiler.py b/packages/reflex-base/src/reflex_base/plugins/compiler.py index 5720d27aab7..c8ffe6d2711 100644 --- a/packages/reflex-base/src/reflex_base/plugins/compiler.py +++ b/packages/reflex-base/src/reflex_base/plugins/compiler.py @@ -689,6 +689,7 @@ class PageContext(BaseContext): frontend_imports: ParsedImportDict = dataclasses.field(default_factory=dict) output_path: str | None = None output_code: str | None = None + source_module: str | None = None # Stack of ``id(component)`` for components whose subtree is # memoize-suppressed. Populated by ``MemoizeStatefulPlugin`` when it # encounters a ``MemoizationLeaf``-style snapshot boundary and popped on @@ -766,8 +767,13 @@ class CompileContext(BaseContext): # ``MemoizeStatefulPlugin``). memoize_wrappers: dict[str, None] = dataclasses.field(default_factory=dict) # Compiler-generated memo definitions for auto-memoized stateful wrappers. - # Stored as ``Any`` to avoid an import cycle with ``reflex_base.components.memo``. - auto_memo_components: dict[str, Any] = dataclasses.field(default_factory=dict) + # Keyed by ``(tag, source_module)`` so identical-rendering subtrees from + # different user modules each get their own entry and emit into the right + # mirrored memo file. Stored as ``Any`` to avoid an import cycle with + # ``reflex_base.components.memo``. + auto_memo_components: dict[tuple[str, str | None], Any] = dataclasses.field( + default_factory=dict + ) def compile( self, diff --git a/packages/reflex-base/src/reflex_base/utils/imports.py b/packages/reflex-base/src/reflex_base/utils/imports.py index e4ec1935739..7e614f99524 100644 --- a/packages/reflex-base/src/reflex_base/utils/imports.py +++ b/packages/reflex-base/src/reflex_base/utils/imports.py @@ -6,6 +6,16 @@ from collections import defaultdict from collections.abc import Mapping, Sequence +# Absolute import paths beginning with one of these reserved ``.web`` +# subdirectories are rewritten to ``$``-prefixed module specifiers. +ABSOLUTE_IMPORT_PREFIXES = ( + "/utils/", + "/components/", + "/styles/", + "/public/", + "/app_components/", +) + def merge_parsed_imports( *imports: ImmutableParsedImportDict, @@ -42,11 +52,7 @@ def merge_imports( import_dict if isinstance(import_dict, tuple) else import_dict.items() ): # If the lib is an absolute path, we need to prefix it with a $ - lib = ( - "$" + lib - if lib.startswith(("/utils/", "/components/", "/styles/", "/public/")) - else lib - ) + lib = "$" + lib if lib.startswith(ABSOLUTE_IMPORT_PREFIXES) else lib if isinstance(fields, (list, tuple, set)): all_imports[lib].extend( ImportVar(field) if isinstance(field, str) else field diff --git a/packages/reflex-base/src/reflex_base/utils/memo_paths.py b/packages/reflex-base/src/reflex_base/utils/memo_paths.py new file mode 100644 index 00000000000..52952a7252b --- /dev/null +++ b/packages/reflex-base/src/reflex_base/utils/memo_paths.py @@ -0,0 +1,280 @@ +"""Mirror user-app Python module paths into the compiler's ``.web`` output. + +The compiler uses these helpers to write each memo's compiled JSX to a path +under ``.web/app_components/`` that mirrors its Python source module, instead of +bundling everything into one file. This module owns the small set of helpers +that: + +- Read ``fn.__module__`` and reject framework / synthetic modules. +- Walk the live frame stack as a fallback for entry points that don't take a + user-supplied callable (notably ``app.add_page(component)`` with a Component + instance). +- Translate a dotted Python module name into mirrored JSX path segments and + the corresponding ``$/...`` library specifier consumed by the import system. +""" + +from __future__ import annotations + +import importlib.util +import inspect +import sys +from collections.abc import Callable +from pathlib import Path + +from reflex_base.constants.base import Dirs + +# Framework packages: a memo defined in one of these (the package itself or any +# submodule below it) is never mirrored, so we don't emit ``.web/reflex/...`` +# files for memos defined inside the framework's own component packages. +_FRAMEWORK_PACKAGES = ( + "reflex", + "reflex_base", + "reflex_site_shared", + "reflex_hosting_cli", + "reflex_docgen", +) + +# Bare-name prefixes matched against the whole module name (no dot boundary), +# covering families of framework packages such as ``reflex_components_radix``. +_FRAMEWORK_NAME_PREFIXES = ("reflex_components_",) + + +def _is_framework_module(module_name: str) -> bool: + """Whether ``module_name`` belongs to the framework itself. + + Args: + module_name: The dotted module name. + + Returns: + True if the module is part of the framework and should not be + mirrored under ``.web/``. + """ + if module_name.startswith(_FRAMEWORK_NAME_PREFIXES): + return True + return any( + module_name == pkg or module_name.startswith(pkg + ".") + for pkg in _FRAMEWORK_PACKAGES + ) + + +def capture_source_module(fn: Callable | None) -> str | None: + """Return the user-app module name for ``fn``, or ``None`` if not user code. + + Reads ``fn.__module__`` directly — Python sets this on every function + definition, and it survives re-exports, decorators that ``functools.wraps`` + correctly, and aliasing. Returns ``None`` for ``__main__``, missing + modules, and framework modules. + + Args: + fn: The user callable whose definition module is wanted. + + Returns: + The dotted module name to mirror under ``.web/app_components/``, or + ``None`` to fall back to the per-name un-mirrored output path. + """ + if fn is None: + return None + module_name = getattr(fn, "__module__", None) + if not module_name or module_name == "__main__": + return None + if _is_framework_module(module_name): + return None + return module_name + + +def resolve_user_module_from_frame(skip: int = 0) -> str | None: + """Walk the live frame stack and return the first user-app module name. + + Used only as a fallback for ``app.add_page(component)`` when the caller + passed a pre-built ``Component`` instance instead of a callable, so there + is no ``__module__`` to read directly. + + Args: + skip: Number of frames above the immediate caller to skip before + starting the search. Pass ``1`` to ignore the function that is + calling this helper. + + Returns: + The first frame's module name that is not a framework module, or + ``None`` if no suitable frame exists (e.g. running inside a REPL). + """ + frame = inspect.currentframe() + if frame is None: + return None + frame = frame.f_back + for _ in range(skip): + if frame is None: + return None + frame = frame.f_back + while frame is not None: + module_name = frame.f_globals.get("__name__") + if ( + module_name + and module_name != "__main__" + and not _is_framework_module(module_name) + ): + return module_name + frame = frame.f_back + return None + + +# Reserved device names on Windows. A file named like one of these (in any +# case, with or without an extension) can't be created normally, so modules +# with such a segment fall back to the un-mirrored output path. +_WINDOWS_RESERVED_NAMES = frozenset({ + "con", + "prn", + "aux", + "nul", + *(f"com{i}" for i in range(1, 10)), + *(f"lpt{i}" for i in range(1, 10)), +}) + + +def _segment_is_safe(segment: str) -> bool: + """Whether ``segment`` is a path-safe Python identifier-like fragment. + + Args: + segment: A single dotted-module segment. + + Returns: + True if the segment can be used as a directory or filename without + introducing path traversal or platform-specific quirks. + """ + if not segment or segment in {".", ".."}: + return False + if any(ch in segment for ch in ("/", "\\", ":", "\0")): + return False + # Windows silently strips trailing dots/spaces and reserves device names, + # either of which breaks the module<->file path correspondence there. + if segment != segment.rstrip(". "): + return False + return segment.casefold() not in _WINDOWS_RESERVED_NAMES + + +def module_to_mirrored_segments(module_name: str | None) -> tuple[str, ...] | None: + """Translate a dotted module name to a tuple of mirrored path segments. + + For a *package* (a module whose import resolves to ``__init__.py``), an + extra ``"index"`` segment is appended so the file lives at + ``/index.jsx`` and submodule files can coexist alongside it as + siblings under ``/``. + + Args: + module_name: The dotted Python module name. ``None`` short-circuits. + + Returns: + A tuple of safe path segments to join under ``.web/app_components/``, or + ``None`` if the module name is missing, contains unsafe segments, or + cannot be resolved as a package vs. module. + """ + if not module_name: + return None + segments = module_name.split(".") + if not all(_segment_is_safe(seg) for seg in segments): + return None + # Prefer the live module's __file__ over a fresh find_spec lookup. A user + # can switch a module to a package (or back) between hot-reload compiles, + # and importlib re-binds __file__ when that happens — a cached find_spec + # result wouldn't. + origin: str | None = None + module = sys.modules.get(module_name) + if module is not None: + origin = getattr(module, "__file__", None) + if origin is None: + try: + spec = importlib.util.find_spec(module_name) + except (ImportError, ValueError): + spec = None + if spec is not None: + origin = spec.origin + if origin and origin.endswith("__init__.py"): + return (*segments, "index") + return tuple(segments) + + +def library_specifier_for(source_module: str | None) -> str | None: + """Return the ``$/...`` import specifier mirroring ``source_module``, or None. + + Args: + source_module: The dotted module name a memo was defined in. + + Returns: + The ``$/`` specifier, or ``None`` if no source module was + captured or it can't be safely mirrored. + """ + if source_module is None: + return None + segments = module_to_mirrored_segments(source_module) + if segments is None: + return None + return mirrored_library_specifier(segments) + + +def mirrored_jsx_path(web_dir: Path, segments: tuple[str, ...]) -> Path: + """Build the absolute ``.jsx`` path under ``web_dir`` for ``segments``. + + Mirrored memos live under the reserved ``app_components/`` subdirectory so a + user module path can never collide with framework output (e.g. a memo in + module ``app.root`` would otherwise overwrite ``.web/app/root.jsx``). + + Args: + web_dir: The project's ``.web`` directory. + segments: Mirrored path segments from + :func:`module_to_mirrored_segments`. + + Returns: + The absolute path the compiler should write the memo module to. + """ + return web_dir.joinpath(Dirs.APP_COMPONENTS, *segments).with_suffix(".jsx") + + +def mirrored_library_specifier(segments: tuple[str, ...]) -> str: + """Build the ``$/...`` import specifier for mirrored ``segments``. + + The specifier has no extension; Vite resolves the ``.jsx`` automatically. It + mirrors :func:`mirrored_jsx_path`, including the reserved ``app_components/`` + subdirectory. + + Args: + segments: Mirrored path segments from + :func:`module_to_mirrored_segments`. + + Returns: + A ``$/`` prefixed module specifier suitable for use as a + ``Component.library`` value. + """ + return "$/" + "/".join((Dirs.APP_COMPONENTS, *segments)) + + +def unmirrored_library_specifier(name: str) -> str: + """Build the ``$/...`` import specifier for an un-mirrorable memo. + + Memos that can't be mirrored to a user module (framework components, + ``__main__``, unsafe names) get one file per memo under the reserved + ``app_components/_internal/`` subdirectory. + + Args: + name: The memo's export name. + + Returns: + A ``$/`` prefixed module specifier suitable for use as a + ``Component.library`` value. + """ + return f"$/{Dirs.APP_COMPONENTS_INTERNAL}/{name}" + + +def library_for(source_module: str | None, name: str) -> str: + """Return the library specifier a memo should import from. + + Mirrors ``source_module`` when it can be safely mirrored, otherwise falls + back to the per-name ``app_components/_internal/`` path. + + Args: + source_module: The dotted module name a memo was defined in. + name: The memo's export name, used for the un-mirrored fallback. + + Returns: + A ``$/`` prefixed module specifier for the memo's compiled output. + """ + return library_specifier_for(source_module) or unmirrored_library_specifier(name) diff --git a/pyi_hashes.json b/pyi_hashes.json index dc601e746f3..ed5d562ba20 100644 --- a/pyi_hashes.json +++ b/pyi_hashes.json @@ -120,5 +120,5 @@ "packages/reflex-components-sonner/src/reflex_components_sonner/toast.pyi": "58521fcd1b514804f534d97624e82c9a", "reflex/__init__.pyi": "56385a4f0d9431eb0056dbc5553a58f9", "reflex/components/__init__.pyi": "9facd05a776d0641432696bbf8e34388", - "reflex/experimental/memo.pyi": "58d97de180cc34a8aa605cd76d97ee57" + "reflex/experimental/memo.pyi": "05e66632984d49c21e60eefd567fc078" } diff --git a/reflex/app.py b/reflex/app.py index 9c0d2af8db0..86a33155bc1 100644 --- a/reflex/app.py +++ b/reflex/app.py @@ -44,7 +44,7 @@ from reflex_base.event.processor import BaseStateEventProcessor, EventProcessor from reflex_base.registry import RegistrationContext from reflex_base.telemetry_context import CompileTrigger, TelemetryContext -from reflex_base.utils import console +from reflex_base.utils import console, memo_paths from reflex_base.utils.imports import ImportVar from reflex_base.utils.types import ASGIApp, Message, Receive, Scope, Send from reflex_components_core.base.error_boundary import ErrorBoundary @@ -283,6 +283,7 @@ class UnevaluatedPage: on_load: EventType[()] | None = None meta: Sequence[Mapping[str, Any] | Component] = () context: Mapping[str, Any] = dataclasses.field(default_factory=dict) + _source_module: str | None = None def merged_with(self, other: UnevaluatedPage) -> UnevaluatedPage: """Merge the other page into this one. @@ -301,6 +302,9 @@ def merged_with(self, other: UnevaluatedPage) -> UnevaluatedPage: 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, + _source_module=self._source_module + if self._source_module is not None + else other._source_module, ) @@ -915,6 +919,13 @@ def add_page( # Check if the route given is valid verify_route_validity(route) + if isinstance(component, Callable): + source_module = memo_paths.capture_source_module(component) + else: + # The user passed a pre-built Component instance — fall back to + # walking the call stack from add_page's caller. + source_module = memo_paths.resolve_user_module_from_frame(skip=1) + unevaluated_page = UnevaluatedPage( component=component, route=route, @@ -924,6 +935,7 @@ def add_page( on_load=on_load, meta=meta, context=context or {}, + _source_module=source_module, ) if route in self._unevaluated_pages: diff --git a/reflex/compiler/compiler.py b/reflex/compiler/compiler.py index 31ce5166ab0..01c43e56dd7 100644 --- a/reflex/compiler/compiler.py +++ b/reflex/compiler/compiler.py @@ -2,6 +2,8 @@ from __future__ import annotations +import collections +import dataclasses import json import sys from collections.abc import Callable, Iterable, Sequence @@ -22,6 +24,7 @@ MemoDefinition, MemoFunctionDefinition, create_component_memo, + reset_memo_component_classes, ) from reflex_base.config import get_config from reflex_base.constants.compiler import PageNames, ResetStylesheet @@ -29,9 +32,10 @@ from reflex_base.environment import environment from reflex_base.plugins import CompileContext, CompilerHooks, PageContext, Plugin from reflex_base.style import SYSTEM_COLOR_MODE +from reflex_base.utils import memo_paths from reflex_base.utils.exceptions import ReflexError from reflex_base.utils.format import to_title_case -from reflex_base.utils.imports import ImportVar +from reflex_base.utils.imports import ABSOLUTE_IMPORT_PREFIXES, ImportVar from reflex_base.vars.base import LiteralVar, Var from reflex_components_core.base.app_wrap import AppWrap from reflex_components_core.base.fragment import Fragment @@ -81,11 +85,7 @@ def _extend_imports_in_place( for lib, fields in ( import_dict if isinstance(import_dict, tuple) else import_dict.items() ): - lib = ( - "$" + lib - if lib.startswith(("/utils/", "/components/", "/styles/", "/public/")) - else lib - ) + lib = "$" + lib if lib.startswith(ABSOLUTE_IMPORT_PREFIXES) else lib target_fields = target.setdefault(lib, []) if isinstance(fields, (list, tuple, set)): target_fields.extend( @@ -399,49 +399,152 @@ def _compile_component(component: Component) -> str: return templates.component_template(component=component) +@dataclasses.dataclass +class _MemoGroup: + """Accumulator for memos that share a mirrored output path.""" + + components: list[dict] = dataclasses.field(default_factory=list) + functions: list[dict] = dataclasses.field(default_factory=list) + imports: dict[str, list[ImportVar]] = dataclasses.field(default_factory=dict) + dynamic_imports: list[str] = dataclasses.field(default_factory=list) + custom_code: list[str] = dataclasses.field(default_factory=list) + + def add_component( + self, render: dict, memo_imports: dict[str, list[ImportVar]] + ) -> None: + self.components.append(render) + _extend_imports_in_place(self.imports, memo_imports) + self.dynamic_imports.extend(sorted(render.get("dynamic_imports", []) or [])) + self.custom_code.extend(render.get("custom_code", []) or []) + + def add_function( + self, render: dict, memo_imports: dict[str, list[ImportVar]] + ) -> None: + self.functions.append(render) + _extend_imports_in_place(self.imports, memo_imports) + + +# Imports every memo module needs regardless of its body: ``memo`` from React +# for the wrapper, and ``isTrue`` for prop coercion. Shared by the grouped and +# un-mirrored compile paths so they can't drift apart. +_MEMO_BASE_IMPORTS: dict[str, list[ImportVar]] = { + "react": [ImportVar(tag="memo")], + f"$/{constants.Dirs.STATE_PATH}": [ImportVar(tag="isTrue")], +} + + def _compile_memo_components( memos: Iterable[MemoDefinition] = (), ) -> tuple[list[tuple[str, str]], dict[str, list[ImportVar]]]: - """Compile each memo as its own module. + """Compile memos grouped by their source module's mirrored output path. - Each memo lands in ``.web//.jsx`` with only the imports - it actually uses. Memo wrappers declare their ``library`` as that - per-memo file path so page-side imports resolve directly to the - individual module. + Memos that captured a user-app source module land in a single combined + file at ``.web/app_components/.jsx`` so the page-side import + surface matches the source layout. Memos that can't be mirrored (framework + components, ``__main__``, unsafe module names) get one file per memo at + ``.web/app_components/_internal/.jsx`` that page-side code imports + directly. Args: memos: The memos to compile. Returns: - A list of ``(path, code)`` pairs to write — one per memo — and the - aggregated imports across all memo modules. + A list of ``(path, code)`` pairs to write and the aggregated imports + across all memo modules. """ - per_memo_files: list[tuple[str, str]] = [] + output_files: list[tuple[str, str]] = [] aggregate_imports: dict[str, list[ImportVar]] = {} + unmirrored_files: list[tuple[str, str]] = [] + unmirrored_base_dir = utils.get_memo_components_dir() + # The subdirectory under ``app_components/`` that holds un-mirrored per-memo + # files; a user module must never mirror into it (see the collision check). + internal_subdir = constants.Dirs.APP_COMPONENTS_INTERNAL.rsplit("/", 1)[-1] + groups: collections.defaultdict[tuple[str, ...], _MemoGroup] = ( + collections.defaultdict(_MemoGroup) + ) - base_dir = utils.get_memo_components_dir() + def _emit_unmirrored( + compile_fn: Callable[[dict, dict], tuple[str, dict[str, list[ImportVar]]]], + render: dict, + render_imports: dict, + ) -> None: + code, file_imports = compile_fn(render, render_imports) + unmirrored_files.append(( + _memo_component_file_path(unmirrored_base_dir, render["name"]), + code, + )) + _extend_imports_in_place(aggregate_imports, file_imports) for memo in memos: if isinstance(memo, MemoComponentDefinition): memo_render, memo_imports = utils.compile_experimental_component_memo(memo) - name = memo_render["name"] - code, file_imports = _compile_single_memo_component( - memo_render, memo_imports - ) - path = _memo_component_file_path(base_dir, name) - per_memo_files.append((path, code)) - _extend_imports_in_place(aggregate_imports, file_imports) + segments = memo_paths.module_to_mirrored_segments(memo.source_module) + if segments is None: + _emit_unmirrored( + _compile_single_memo_component, memo_render, memo_imports + ) + else: + groups[segments].add_component(memo_render, memo_imports) elif isinstance(memo, MemoFunctionDefinition): memo_render, memo_imports = utils.compile_experimental_function_memo(memo) - name = memo_render["name"] - code, file_imports = _compile_single_memo_function( - memo_render, memo_imports + segments = memo_paths.module_to_mirrored_segments(memo.source_module) + if segments is None: + _emit_unmirrored( + _compile_single_memo_function, memo_render, memo_imports + ) + else: + groups[segments].add_function(memo_render, memo_imports) + + if groups: + _extend_imports_in_place(aggregate_imports, _MEMO_BASE_IMPORTS) + _apply_common_imports(aggregate_imports) + + # Maps a case-folded output path to the module that claimed it, so two + # modules differing only by case (which collide on case-insensitive + # filesystems) are caught instead of one silently overwriting the other. + claimed_paths: dict[str, str] = {} + for segments, group in groups.items(): + module_path = utils.get_memo_module_path(segments) + module_name = ".".join(segments) + # A user module that mirrors into the reserved internal directory would + # overwrite the per-memo files emitted there. Fail loudly rather than + # silently clobbering output. + if segments[0] == internal_subdir: + msg = ( + f"Memoized component module {module_name!r} mirrors into " + f"Reflex's reserved {constants.Dirs.APP_COMPONENTS_INTERNAL!r} " + f"memo output directory. Rename the source module so its " + f"top-level package is not {internal_subdir!r}." + ) + raise ReflexError(msg) + case_key = module_path.casefold() + if (clash := claimed_paths.get(case_key)) is not None: + msg = ( + f"Memoized component modules {clash!r} and {module_name!r} both " + f"mirror to {module_path!r} (their paths differ only by case), " + f"which collides on case-insensitive filesystems. Rename one of " + f"the source modules." ) - path = _memo_component_file_path(base_dir, name) - per_memo_files.append((path, code)) - _extend_imports_in_place(aggregate_imports, file_imports) + raise ReflexError(msg) + claimed_paths[case_key] = module_name + # Strip self-imports — when memos in this group reference each other, + # their compiled imports point at this group's own mirrored specifier. + # Importing from the same file would shadow the module's own exports. + self_specifier = memo_paths.mirrored_library_specifier(segments) + group.imports.pop(self_specifier, None) + merged_imports = utils.merge_imports(_MEMO_BASE_IMPORTS, group.imports) + _apply_common_imports(merged_imports) + code = templates.memo_components_template( + imports=utils.compile_imports(merged_imports), + components=group.components, + functions=group.functions, + dynamic_imports=sorted(set(group.dynamic_imports)), + custom_codes=list(dict.fromkeys(group.custom_code)), + ) + output_files.append((module_path, code)) + _extend_imports_in_place(aggregate_imports, group.imports) - return per_memo_files, aggregate_imports + return [*unmirrored_files, *output_files], aggregate_imports def _compile_single_memo_component( @@ -458,13 +561,7 @@ def _compile_single_memo_component( Returns: The file contents and the full import dict used to compile it. """ - imports = utils.merge_imports( - { - "react": [ImportVar(tag="memo")], - f"$/{constants.Dirs.STATE_PATH}": [ImportVar(tag="isTrue")], - }, - component_imports, - ) + imports = utils.merge_imports(_MEMO_BASE_IMPORTS, component_imports) _apply_common_imports(imports) code = templates.memo_single_component_template( imports=utils.compile_imports(imports), @@ -509,18 +606,6 @@ def _memo_component_file_path(base_dir: str, name: str) -> str: return str(Path(base_dir) / f"{name}{constants.Ext.JSX}") -def _memo_component_index_specifier(name: str) -> str: - """Return the module specifier the index uses to re-export a memo. - - Args: - name: The memo's export name. - - Returns: - A relative specifier resolvable from the memo index module. - """ - return f"./{constants.PageNames.COMPONENTS}/{name}" - - def compile_document_root( head_components: list[Component], html_lang: str | None = None, @@ -1083,6 +1168,10 @@ def compile_app( config.plugins, ) reset_bundled_libraries() + # Drop cached memo wrapper classes so each compile recomputes a memo's + # ``library`` from the current module layout (handles a module flipping to + # a package across hot reloads). + reset_memo_component_classes() for plugin in compiler_plugins: for dependency in plugin.get_frontend_dependencies(): bundle_library(dependency) @@ -1170,9 +1259,9 @@ def compile_app( hydrate_fallback_definition = create_component_memo( hydrate_fallback, "hydrate_fallback" ) - compile_ctx.auto_memo_components[hydrate_fallback_definition.export_name] = ( - hydrate_fallback_definition - ) + compile_ctx.auto_memo_components[ + hydrate_fallback_definition.export_name, None + ] = hydrate_fallback_definition hydrate_fallback_export = hydrate_fallback_definition.export_name memo_component_files, memo_components_imports = compile_memo_components( @@ -1275,6 +1364,10 @@ def add_save_task( if dry_run: return True + # Delete memo files this compile no longer emits. Done here (not before the + # dry-run return) so ``--dry`` never mutates ``.web`` or the manifest. + utils.prune_stale_memo_files(path for path, _ in memo_component_files) + with console.timing("Install Frontend Packages"): app._get_frontend_packages(all_imports) diff --git a/reflex/compiler/plugins/builtin.py b/reflex/compiler/plugins/builtin.py index de31f8c83c7..4081b4b9d79 100644 --- a/reflex/compiler/plugins/builtin.py +++ b/reflex/compiler/plugins/builtin.py @@ -205,6 +205,7 @@ def eval_page( name=getattr(page_fn, "__name__", page.route), route=page.route, root_component=component, + source_module=getattr(page, "_source_module", None), ) diff --git a/reflex/compiler/plugins/memoize.py b/reflex/compiler/plugins/memoize.py index 3eb4bc51873..41c4bab97ae 100644 --- a/reflex/compiler/plugins/memoize.py +++ b/reflex/compiler/plugins/memoize.py @@ -10,7 +10,7 @@ Each unique subtree shape contributes: - One generated experimental memo component definition, compiled into its own - per-memo module at ``$/utils/components/``. + per-memo module at ``$/app_components/_internal/``. - ``useCallback`` hook lines for each non-lifecycle event trigger, emitted into the generated memo body so handler hooks stay inside that rendering domain. @@ -381,10 +381,15 @@ def _build_wrapper( # ``create_passthrough_component_memo`` after the ``{children}`` hole # is substituted, so passthrough wrappers that differ only in their # children collapse to a single definition. - wrapper_factory, definition = create_passthrough_component_memo(comp) + wrapper_factory, definition = create_passthrough_component_memo( + comp, source_module=page_context.source_module + ) tag = definition.export_name compile_context.memoize_wrappers[tag] = None - compile_context.auto_memo_components[tag] = definition + # Key by (tag, source_module) so identical-rendering subtrees from + # different user modules each get their own entry and emit into the + # right mirrored memo file. + compile_context.auto_memo_components[tag, definition.source_module] = definition wrapper = wrapper_factory() # The wrapper has no structural children at the page level, but parents diff --git a/reflex/compiler/utils.py b/reflex/compiler/utils.py index 6daf1390a44..53fb8729e1e 100644 --- a/reflex/compiler/utils.py +++ b/reflex/compiler/utils.py @@ -5,9 +5,12 @@ import asyncio import concurrent.futures import copy +import json import operator +import os +import tempfile import traceback -from collections.abc import Mapping, Sequence +from collections.abc import Iterable, Mapping, Sequence from datetime import datetime from pathlib import Path from typing import Any, TypedDict @@ -22,7 +25,7 @@ ) from reflex_base.constants.state import FIELD_MARKER from reflex_base.style import Style -from reflex_base.utils import format, imports +from reflex_base.utils import format, imports, memo_paths from reflex_base.utils.imports import ImportVar, ParsedImportDict from reflex_base.vars.base import Field, Var, VarData from reflex_base.vars.function import DestructuredArg @@ -420,10 +423,10 @@ def compile_experimental_component_memo( dynamic_imports = render._get_all_dynamic_imports() all_imports = render._get_all_imports() - # Each memo lives in ``web/utils/components/.jsx`` and is imported - # from ``$/utils/components/``. Strip a self-import so a memo body - # that references the wrapper's own module specifier doesn't recurse. - self_module = f"$/{constants.Dirs.COMPONENTS_PATH}/{definition.export_name}" + # Each un-mirrored memo lives in ``web/app_components/_internal/.jsx`` + # and is imported from ``$/app_components/_internal/``. Strip a + # self-import so a memo body that references its own specifier doesn't recurse. + self_module = memo_paths.unmirrored_library_specifier(definition.export_name) imports: ParsedImportDict = { lib: fields for lib, fields in all_imports.items() if lib != self_module } @@ -529,9 +532,9 @@ def compile_experimental_function_memo( # Reading ``.function`` evaluates a deferred function-memo body on first use. function = definition.function if var_data := function._get_all_var_data(): - # Per-file memo modules live at ``$/utils/components/``; strip - # only a self-import to this function memo's own module. - self_module = f"$/{constants.Dirs.COMPONENTS_PATH}/{definition.python_name}" + # Per-file memo modules live at ``$/app_components/_internal/``; + # strip only a self-import to this function memo's own module. + self_module = memo_paths.unmirrored_library_specifier(definition.python_name) imports = { lib: list(fields) for lib, fields in dict(var_data.imports).items() @@ -748,11 +751,22 @@ def get_memo_components_dir() -> str: Returns: The directory used for per-memo ``.jsx`` modules. Pages import each - wrapper directly from ``$/utils/components/``. + wrapper directly from ``$/app_components/_internal/``. """ - return str( - get_web_dir() / constants.Dirs.UTILS / constants.PageNames.COMPONENTS, - ) + return str(get_web_dir() / constants.Dirs.APP_COMPONENTS_INTERNAL) + + +def get_memo_module_path(segments: tuple[str, ...]) -> str: + """Get the on-disk path for a memo module mirrored from a Python module. + + Args: + segments: Mirrored path segments produced by + :func:`reflex_base.utils.memo_paths.module_to_mirrored_segments`. + + Returns: + The absolute path the compiler should write the combined memo file to. + """ + return str(memo_paths.mirrored_jsx_path(get_web_dir(), segments)) def add_meta( @@ -819,6 +833,97 @@ def write_file(path: str | Path, code: str): path.write_text(code, encoding="utf-8") +_MEMO_MANIFEST_FILENAME = ".memo-manifest.json" + + +def _read_memo_manifest(web_dir: Path) -> set[str]: + """Read the previous compile's memo file manifest. + + Args: + web_dir: The project's ``.web`` directory. + + Returns: + The set of paths (relative to ``.web``) recorded by the previous + compile, or an empty set if the manifest is absent or invalid. + """ + manifest_path = web_dir / _MEMO_MANIFEST_FILENAME + if not manifest_path.exists(): + return set() + try: + data = json.loads(manifest_path.read_text(encoding="utf-8")) + except (OSError, ValueError): + return set() + if not isinstance(data, list): + return set() + return {entry for entry in data if isinstance(entry, str)} + + +def _write_memo_manifest(web_dir: Path, relative_paths: set[str]) -> None: + """Atomically write the new memo file manifest. + + Args: + web_dir: The project's ``.web`` directory. + relative_paths: Paths emitted this run, relative to ``.web``. + """ + manifest_path = web_dir / _MEMO_MANIFEST_FILENAME + web_dir.mkdir(parents=True, exist_ok=True) + fd, tmp_name = tempfile.mkstemp( + prefix=".memo-manifest.", suffix=".json.tmp", dir=str(web_dir) + ) + # Close the raw fd immediately and reopen the file by path. Wrapping the + # fd via os.fdopen() would leak it if the wrap itself raised. + os.close(fd) + tmp_path = Path(tmp_name) + try: + with tmp_path.open("w", encoding="utf-8") as fh: + json.dump(sorted(relative_paths), fh) + tmp_path.replace(manifest_path) + except Exception: + # Best-effort cleanup; manifest write is recoverable on the next run. + tmp_path.unlink(missing_ok=True) + raise + + +def prune_stale_memo_files(emitted_paths: Iterable[str | Path]) -> None: + """Delete memo files written previously that this compile no longer emits. + + Only paths that appear in the previous manifest are considered for + deletion — never a fresh filesystem walk — so files this code did not + emit are never touched. Empty parent directories created by mirrored + output are removed up to (but not including) the ``.web`` root. + + Args: + emitted_paths: Paths the current compile produced for the memo + pipeline, as built by joining :func:`get_web_dir` (so they share + its prefix — relative by default, absolute when overridden). + """ + web_dir = get_web_dir() + + emitted_relative: set[str] = set() + for path in emitted_paths: + try: + relative = Path(path).relative_to(web_dir) + except ValueError: + continue + emitted_relative.add(str(relative).replace(os.sep, "/")) + + previous = _read_memo_manifest(web_dir) + for relative in previous - emitted_relative: + target = web_dir / relative + if target.is_file(): + target.unlink() + parent = target.parent + while parent != web_dir and parent.is_relative_to(web_dir): + try: + parent.rmdir() + except OSError: + break + parent = parent.parent + + if emitted_relative != previous: + _write_memo_manifest(web_dir, emitted_relative) + + def empty_dir(path: str | Path, keep_files: list[str] | None = None): """Remove all files and folders in a directory except for the keep_files. diff --git a/tests/integration/test_auto_memo.py b/tests/integration/test_auto_memo.py index 121184f493e..25601755f17 100644 --- a/tests/integration/test_auto_memo.py +++ b/tests/integration/test_auto_memo.py @@ -55,7 +55,7 @@ def test_auto_memo_shared_across_pages(auto_memo_app: AppHarness): web_sources = "\n".join( path.read_text() for path in (auto_memo_app.app_path / ".web").rglob("*.jsx") ) - assert "$/utils/components" in web_sources + assert "$/app_components" in web_sources assert "$/utils/stateful_components" not in web_sources driver = auto_memo_app.frontend() diff --git a/tests/units/compiler/test_compiler.py b/tests/units/compiler/test_compiler.py index 1dbb4ab27bc..5146f506264 100644 --- a/tests/units/compiler/test_compiler.py +++ b/tests/units/compiler/test_compiler.py @@ -399,7 +399,7 @@ def test_compile_app_root_with_hydrate_fallback_exports_hydrate_fallback(): assert ( "export { MyFallback as HydrateFallback } " - 'from "$/utils/components/MyFallback";' in code + 'from "$/app_components/_internal/MyFallback";' in code ) diff --git a/tests/units/compiler/test_memoize_plugin.py b/tests/units/compiler/test_memoize_plugin.py index 19c25c42fb7..01bfa181804 100644 --- a/tests/units/compiler/test_memoize_plugin.py +++ b/tests/units/compiler/test_memoize_plugin.py @@ -133,6 +133,7 @@ class FakePage: description: Any = None image: str = "" meta: tuple[dict[str, Any], ...] = () + _source_module: str | None = None def _compile_single_page( @@ -217,9 +218,12 @@ def test_memoize_wrapper_uses_memo_component_and_call_site() -> None: assert len(ctx.memoize_wrappers) == 1 wrapper_tag = next(iter(ctx.memoize_wrappers)) - assert wrapper_tag in ctx.auto_memo_components + assert (wrapper_tag, None) in ctx.auto_memo_components output = page_ctx.output_code or "" - assert f'import {{{wrapper_tag}}} from "$/utils/components/{wrapper_tag}"' in output + assert ( + f'import {{{wrapper_tag}}} from "$/app_components/_internal/{wrapper_tag}"' + in output + ) assert f"jsx({wrapper_tag}," in (page_ctx.output_code or "") assert f"const {wrapper_tag} = memo" not in output @@ -245,9 +249,9 @@ def test_memoize_wrapper_deduped_across_repeated_subtrees() -> None: ) assert len(ctx.memoize_wrappers) == 1 wrapper_tag = next(iter(ctx.memoize_wrappers)) - assert list(ctx.auto_memo_components) == [wrapper_tag] + assert list(ctx.auto_memo_components) == [(wrapper_tag, None)] assert (page_ctx.output_code or "").count( - f'import {{{wrapper_tag}}} from "$/utils/components/{wrapper_tag}"' + f'import {{{wrapper_tag}}} from "$/app_components/_internal/{wrapper_tag}"' ) == 1 @@ -615,8 +619,14 @@ def test_passthrough_memo_definitions_are_not_shared_globally(monkeypatch) -> No lambda comp, page_context: comp, ) - def fake_create_passthrough_component_memo(component: Component): - definition = SimpleNamespace(export_name=tag, component=component) + def fake_create_passthrough_component_memo( + component: Component, source_module: str | None = None + ): + definition = SimpleNamespace( + export_name=tag, + component=component, + source_module=source_module, + ) return (lambda definition=definition: definition), definition monkeypatch.setattr( @@ -627,7 +637,7 @@ def fake_create_passthrough_component_memo(component: Component): first_compile = SimpleNamespace(memoize_wrappers={}, auto_memo_components={}) second_compile = SimpleNamespace(memoize_wrappers={}, auto_memo_components={}) - page_context = cast(PageContext, SimpleNamespace()) + page_context = cast(PageContext, SimpleNamespace(source_module=None)) MemoizeStatefulPlugin._build_wrapper( first_component, @@ -640,8 +650,8 @@ def fake_create_passthrough_component_memo(component: Component): compile_context=second_compile, ) - first_definition = first_compile.auto_memo_components[tag] - second_definition = second_compile.auto_memo_components[tag] + first_definition = first_compile.auto_memo_components[tag, None] + second_definition = second_compile.auto_memo_components[tag, None] assert first_definition.component is first_component assert second_definition.component is second_component assert second_definition is not first_definition @@ -661,13 +671,80 @@ def test_shared_subtree_across_pages_uses_same_tag() -> None: assert len(ctx.memoize_wrappers) == 1 tag = next(iter(ctx.memoize_wrappers)) - assert list(ctx.auto_memo_components) == [tag] + assert list(ctx.auto_memo_components) == [(tag, None)] for route in ("/a", "/b"): output = ctx.compiled_pages[route].output_code or "" - assert f'import {{{tag}}} from "$/utils/components/{tag}"' in output + assert f'import {{{tag}}} from "$/app_components/_internal/{tag}"' in output assert f"jsx({tag}," in output +def test_shared_subtree_in_distinct_source_modules_emits_per_module() -> None: + """Identical subtrees in different user modules emit one memo per module. + + Regression: the auto-memo registry was keyed by tag only, so when two + pages from different user modules produced the same memoizable subtree + (and therefore the same tag), the second registration overwrote the + first. Only the surviving definition's source module got a mirrored + memo file, leaving the other page importing a tag from a file that + never exported it. + """ + from reflex.compiler.compiler import compile_memo_components + + ctx = CompileContext( + pages=[ + FakePage( + route="/a", + component=lambda: Plain.create(STATE_VAR), + _source_module="memo_collision_test.module_a", + ), + FakePage( + route="/b", + component=lambda: Plain.create(STATE_VAR), + _source_module="memo_collision_test.module_b", + ), + ], + hooks=CompilerHooks(plugins=default_page_plugins()), + ) + with ctx: + ctx.compile() + + assert len(ctx.memoize_wrappers) == 1 + tag = next(iter(ctx.memoize_wrappers)) + # Both source modules survive registration as distinct entries. + assert set(ctx.auto_memo_components) == { + (tag, "memo_collision_test.module_a"), + (tag, "memo_collision_test.module_b"), + } + + page_a_output = ctx.compiled_pages["/a"].output_code or "" + page_b_output = ctx.compiled_pages["/b"].output_code or "" + assert ( + f'import {{{tag}}} from "$/app_components/memo_collision_test/module_a"' + in page_a_output + ) + assert ( + f'import {{{tag}}} from "$/app_components/memo_collision_test/module_b"' + in page_b_output + ) + + output_files, _ = compile_memo_components( + memos=tuple(ctx.auto_memo_components.values()), + ) + emitted = {Path(path).as_posix(): code for path, code in output_files} + + def find_emitted(suffix: str) -> str | None: + return next( + (code for path, code in emitted.items() if path.endswith(suffix)), None + ) + + matched_a = find_emitted("memo_collision_test/module_a.jsx") + matched_b = find_emitted("memo_collision_test/module_b.jsx") + assert matched_a is not None, f"missing module_a memo file in {sorted(emitted)}" + assert matched_b is not None, f"missing module_b memo file in {sorted(emitted)}" + assert f"export const {tag} = memo" in matched_a + assert f"export const {tag} = memo" in matched_b + + def test_shared_parent_instance_across_pages_preserves_original() -> None: """A parent instance reused across pages must not have its children rebound. @@ -702,7 +779,7 @@ def test_shared_parent_instance_across_pages_preserves_original() -> None: tag = next(iter(ctx.memoize_wrappers)) for route in ("/a", "/b"): output = ctx.compiled_pages[route].output_code or "" - assert f'import {{{tag}}} from "$/utils/components/{tag}"' in output, ( + assert f'import {{{tag}}} from "$/app_components/_internal/{tag}"' in output, ( f"route {route} missing memo tag import" ) assert f"jsx({tag}," in output, f"route {route} does not render the memo tag" @@ -747,7 +824,7 @@ def test_shared_nested_parent_mirroring_common_elements_preserves_original() -> tag = next(iter(ctx.memoize_wrappers)) for route in ("/a", "/b", "/c"): output = ctx.compiled_pages[route].output_code or "" - assert f'import {{{tag}}} from "$/utils/components/{tag}"' in output + assert f'import {{{tag}}} from "$/app_components/_internal/{tag}"' in output assert f"jsx({tag}," in output @@ -2206,7 +2283,7 @@ def test_restricted_content_element_with_id_and_stateful_child_still_memoizes() def test_each_memo_wrapper_emits_one_component_module_file() -> None: - """Every wrapper tag corresponds to exactly one ``components/{tag}.jsx`` file. + """Every wrapper tag corresponds to exactly one ``app_components/_internal/{tag}.jsx`` file. Locks the per-wrapper file invariant: ``compile_memo_components`` must emit one module per wrapper (plus the shared index), so that React can @@ -2228,7 +2305,7 @@ def test_each_memo_wrapper_emits_one_component_module_file() -> None: component_module_names = { Path(path).name for path, _ in memo_files - if Path(path).parent.name == "components" + if Path(path).parent.name == "_internal" } expected = {f"{tag}.jsx" for tag in ctx.memoize_wrappers} assert component_module_names == expected, ( diff --git a/tests/units/compiler/test_plugins.py b/tests/units/compiler/test_plugins.py index 08943270a3b..107b2b1d20b 100644 --- a/tests/units/compiler/test_plugins.py +++ b/tests/units/compiler/test_plugins.py @@ -1172,11 +1172,11 @@ def test_compile_context_memoize_wrappers_registers_shared_subtree_tag() -> None # Both pages share the same subtree hash, so exactly one wrapper tag is registered. assert len(compile_ctx.memoize_wrappers) == 1 wrapper_tag = next(iter(compile_ctx.memoize_wrappers)) - assert list(compile_ctx.auto_memo_components) == [wrapper_tag] + assert [key for key, _ in compile_ctx.auto_memo_components] == [wrapper_tag] # Each page imports the generated experimental memo component. page_a_code = compile_ctx.compiled_pages["/a"].output_code or "" assert ( - f'import {{{wrapper_tag}}} from "$/utils/components/{wrapper_tag}"' + f'import {{{wrapper_tag}}} from "$/app_components/_internal/{wrapper_tag}"' in page_a_code ) assert f"jsx({wrapper_tag}," in page_a_code @@ -1194,7 +1194,7 @@ def test_compile_context_resets_memoize_wrappers_between_runs() -> None: with ctx: ctx.compile() first_tags = set(ctx.memoize_wrappers) - first_defs = set(ctx.auto_memo_components) + first_defs = {tag for tag, _ in ctx.auto_memo_components} assert first_tags # memoize wrapper was registered assert first_defs == first_tags @@ -1208,7 +1208,7 @@ def test_compile_context_resets_memoize_wrappers_between_runs() -> None: # Same shared component → same tag, not a union across runs. assert set(ctx2.memoize_wrappers) == first_tags - assert set(ctx2.auto_memo_components) == first_tags + assert {tag for tag, _ in ctx2.auto_memo_components} == first_tags page_ctx = ctx2.compiled_pages["/c"] assert "react-moment" in page_ctx.frontend_imports assert "$/utils/stateful_components" not in (page_ctx.output_code or "") diff --git a/tests/units/compiler/test_stale_cleanup.py b/tests/units/compiler/test_stale_cleanup.py new file mode 100644 index 00000000000..500fc02861c --- /dev/null +++ b/tests/units/compiler/test_stale_cleanup.py @@ -0,0 +1,169 @@ +"""Tests for the memo manifest-driven stale file cleanup.""" + +from __future__ import annotations + +import json +from pathlib import Path +from unittest.mock import patch + +import pytest + +from reflex.compiler import utils as compiler_utils + + +@pytest.fixture +def fake_web_dir(tmp_path: Path): + """Pretend tmp_path is the project's .web directory. + + Args: + tmp_path: The pytest tmp directory. + + Yields: + The path used as ``.web`` for the duration of the test. + """ + web_dir = tmp_path / ".web" + web_dir.mkdir() + with patch.object(compiler_utils, "get_web_dir", return_value=web_dir): + yield web_dir + + +@pytest.fixture +def relative_web_dir(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + """Use a relative ``.web`` dir, matching the default get_web_dir() value. + + The other tests mock an absolute web dir, which hides path-joining bugs + that only surface when ``.web`` is the default relative path. + + Args: + tmp_path: The pytest tmp directory. + monkeypatch: The pytest monkeypatch fixture. + + Yields: + The relative ``.web`` path used for the duration of the test. + """ + monkeypatch.chdir(tmp_path) + web_dir = Path(".web") + web_dir.mkdir() + with patch.object(compiler_utils, "get_web_dir", return_value=web_dir): + yield web_dir + + +def _seed_manifest(web_dir: Path, paths: list[str]) -> None: + (web_dir / compiler_utils._MEMO_MANIFEST_FILENAME).write_text( + json.dumps(paths), encoding="utf-8" + ) + + +def _seed_files(web_dir: Path, relative_paths: list[str]) -> list[Path]: + written: list[Path] = [] + for rel in relative_paths: + target = web_dir / rel + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text("// memo", encoding="utf-8") + written.append(target) + return written + + +def test_prune_removes_files_dropped_between_runs(fake_web_dir: Path): + survivor, removed = _seed_files( + fake_web_dir, + ["myapp/widgets/buttons.jsx", "myapp/dropped.jsx"], + ) + _seed_manifest(fake_web_dir, ["myapp/widgets/buttons.jsx", "myapp/dropped.jsx"]) + + compiler_utils.prune_stale_memo_files([survivor]) + + assert survivor.exists() + assert not removed.exists() + + +def test_prune_cleans_empty_parent_dirs(fake_web_dir: Path): + survivor, _orphan = _seed_files( + fake_web_dir, + ["myapp/keep.jsx", "myapp/widgets/orphan.jsx"], + ) + _seed_manifest(fake_web_dir, ["myapp/keep.jsx", "myapp/widgets/orphan.jsx"]) + + compiler_utils.prune_stale_memo_files([survivor]) + + assert not (fake_web_dir / "myapp" / "widgets").exists() + assert (fake_web_dir / "myapp").exists() + + +def test_prune_only_touches_manifest_paths(fake_web_dir: Path): + untouched = fake_web_dir / "user_added.jsx" + untouched.write_text("// stays", encoding="utf-8") + [survivor] = _seed_files(fake_web_dir, ["myapp/keep.jsx"]) + # Manifest only mentions the survivor — even if other files exist next to + # it, prune must never delete files outside the manifest's history. + _seed_manifest(fake_web_dir, ["myapp/keep.jsx"]) + + compiler_utils.prune_stale_memo_files([survivor]) + + assert untouched.exists() + assert survivor.exists() + + +def test_prune_writes_new_manifest(fake_web_dir: Path): + [survivor] = _seed_files(fake_web_dir, ["myapp/widgets/buttons.jsx"]) + _seed_manifest(fake_web_dir, []) + + compiler_utils.prune_stale_memo_files([survivor]) + + manifest = json.loads( + (fake_web_dir / compiler_utils._MEMO_MANIFEST_FILENAME).read_text( + encoding="utf-8" + ) + ) + assert manifest == ["myapp/widgets/buttons.jsx"] + + +def test_prune_handles_missing_previous_manifest(fake_web_dir: Path): + [survivor] = _seed_files(fake_web_dir, ["myapp/widgets/buttons.jsx"]) + + # No manifest seeded — should not raise and should still write one. + compiler_utils.prune_stale_memo_files([survivor]) + + assert (fake_web_dir / compiler_utils._MEMO_MANIFEST_FILENAME).exists() + + +def test_prune_handles_relative_web_dir(relative_web_dir: Path): + survivor, renamed_from = _seed_files( + relative_web_dir, + ["myapp/keep.jsx", "myapp/old_name.jsx"], + ) + _seed_manifest(relative_web_dir, ["myapp/keep.jsx", "myapp/old_name.jsx"]) + # The rename target is written fresh this run and was never in the manifest. + [renamed_to] = _seed_files(relative_web_dir, ["myapp/new_name.jsx"]) + + # The compiler emits ``.web``-prefixed paths (str(get_web_dir() / ...)). With + # a relative ``.web`` the old is-absolute guard double-prefixed these to + # ``.web/.web/...``, so emitted keys never matched the manifest: the survivor + # and rename target were wrongly pruned and the new manifest was corrupted. + emitted = [ + str(relative_web_dir / "myapp" / "keep.jsx"), + str(relative_web_dir / "myapp" / "new_name.jsx"), + ] + compiler_utils.prune_stale_memo_files(emitted) + + assert survivor.exists() + assert renamed_to.exists() + assert not renamed_from.exists() + + manifest = json.loads( + (relative_web_dir / compiler_utils._MEMO_MANIFEST_FILENAME).read_text( + encoding="utf-8" + ) + ) + assert manifest == ["myapp/keep.jsx", "myapp/new_name.jsx"] + + +def test_prune_ignores_corrupt_manifest(fake_web_dir: Path): + (fake_web_dir / compiler_utils._MEMO_MANIFEST_FILENAME).write_text( + "not json", encoding="utf-8" + ) + [survivor] = _seed_files(fake_web_dir, ["myapp/widgets/buttons.jsx"]) + + compiler_utils.prune_stale_memo_files([survivor]) + + assert survivor.exists() diff --git a/tests/units/components/test_memo.py b/tests/units/components/test_memo.py index 9276bf248c2..f082dd7c8b8 100644 --- a/tests/units/components/test_memo.py +++ b/tests/units/components/test_memo.py @@ -25,8 +25,9 @@ ) from reflex_base.event import EventChain, EventHandler, no_args_event_spec from reflex_base.style import Style -from reflex_base.utils import console +from reflex_base.utils import console, memo_paths from reflex_base.utils import format as format_utils +from reflex_base.utils.exceptions import ReflexError from reflex_base.utils.imports import ImportVar from reflex_base.vars import VarData from reflex_base.vars.base import Var @@ -414,7 +415,10 @@ def dropper(title: rx.Var[str]) -> rx.Component: component.class_name = Var.create("leaks") # set past the call-site gate files, _ = compiler.compile_memo_components(tuple(MEMOS.values())) - code = next(c for path, c in files if path.endswith("Dropper.jsx")) + segments = memo_paths.module_to_mirrored_segments(__name__) + assert segments is not None + exp_path = compiler_utils.get_memo_module_path(segments) + code = next(c for path, c in files if path == exp_path) # No rest capture, and the root Box gets an empty props object. assert "...rest" not in code assert "className" not in code @@ -434,7 +438,10 @@ def rest_card(rest: rx.RestProp, *, title: rx.Var[str]) -> rx.Component: assert isinstance(component, MemoComponent) files, _ = compiler.compile_memo_components(tuple(MEMOS.values())) - code = next(c for path, c in files if path.endswith("RestCard.jsx")) + segments = memo_paths.module_to_mirrored_segments(__name__) + assert segments is not None + exp_path = compiler_utils.get_memo_module_path(segments) + code = next(c for path, c in files if path == exp_path) # Undeclared props are captured in ``...rest`` and spread onto the root, so # ``className``/``id`` actually reach the rendered element. assert "...rest" in code @@ -893,6 +900,120 @@ def my_card(children: rx.Var[rx.Component], *, title: rx.Var[str]) -> rx.Compone assert "export const MyCard = memo(" in code +def test_compile_memo_components_groups_by_source_module(): + """Memos sharing a source module are concatenated into one mirrored file.""" + + @rx.memo + def grouped_first(title: rx.Var[str]) -> rx.Component: + return rx.text(title) + + @rx.memo + def grouped_second(title: rx.Var[str]) -> rx.Component: + return rx.heading(title) + + definition = MEMOS["GroupedFirst"] + assert definition.source_module is not None + segments = memo_paths.module_to_mirrored_segments(definition.source_module) + assert segments is not None + + files, _ = compiler.compile_memo_components(tuple(MEMOS.values())) + exp_path = compiler_utils.get_memo_module_path(segments) + + grouped_files = [(path, code) for path, code in files if path == exp_path] + assert len(grouped_files) == 1 + code = grouped_files[0][1] + assert "export const GroupedFirst = memo(" in code + assert "export const GroupedSecond = memo(" in code + # The merged module must carry imports its memos use, not just the + # framework-level ones added by the compiler. + assert "RadixThemesText" in code + assert "RadixThemesHeading" in code + + +def test_compile_memo_components_falls_back_when_no_source_module(): + """Memos with no source module emit to the legacy per-name path.""" + legacy_definition = MemoComponentDefinition( + fn=lambda: None, + python_name="legacy_memo", + params=(), + export_name="LegacyMemo", + _component=_LazyBody.ready(rx.fragment()), + passthrough_hole_child=None, + ) + + files, _ = compiler.compile_memo_components((legacy_definition,)) + exp_path = compiler._memo_component_file_path( + compiler_utils.get_memo_components_dir(), "LegacyMemo" + ) + assert any(path == exp_path for path, _ in files) + + +def test_compile_memo_components_rejects_collision_with_reserved_internal_dir(): + """A user module mirroring into the reserved internal dir fails loudly. + + A module whose first segment matches the reserved ``_internal`` directory + would mirror on top of the per-memo files emitted there, so compilation + must raise rather than silently clobber output. + """ + definition = MemoComponentDefinition( + fn=lambda: None, + python_name="collides", + params=(), + export_name="Collides", + _component=_LazyBody.ready(rx.fragment()), + passthrough_hole_child=None, + source_module="_internal.collides", + ) + + with pytest.raises(ReflexError, match="reserved"): + compiler.compile_memo_components((definition,)) + + +def test_compile_memo_components_rejects_single_segment_internal_module(): + """A top-level module literally named ``_internal`` is also rejected. + + The guard must catch the single-segment case (mirroring to + ``app_components/_internal.jsx``), not just multi-segment ``_internal.x``. + """ + definition = MemoComponentDefinition( + fn=lambda: None, + python_name="thing", + params=(), + export_name="Thing", + _component=_LazyBody.ready(rx.fragment()), + passthrough_hole_child=None, + source_module="_internal", + ) + + with pytest.raises(ReflexError, match="reserved"): + compiler.compile_memo_components((definition,)) + + +def test_compile_memo_components_rejects_case_insensitive_path_collision(): + """Two modules whose mirrored paths differ only by case fail loudly. + + On case-insensitive filesystems (macOS/Windows) both would resolve to one + file, silently overwriting one memo module with the other. + """ + + def _definition(export_name: str, source_module: str) -> MemoComponentDefinition: + return MemoComponentDefinition( + fn=lambda: None, + python_name=export_name.lower(), + params=(), + export_name=export_name, + _component=_LazyBody.ready(rx.fragment()), + passthrough_hole_child=None, + source_module=source_module, + ) + + with pytest.raises(ReflexError, match="case"): + compiler.compile_memo_components(( + _definition("Upper", "casecollide.Widget"), + _definition("Lower", "casecollide.widget"), + )) + + def test_compile_memo_components_extends_imports_without_remerging( monkeypatch: pytest.MonkeyPatch, ): @@ -1431,9 +1552,13 @@ def recursive_box(items: rx.Var[list[int]]) -> rx.Component: assert isinstance(definition, MemoComponentDefinition) files, _ = compiler.compile_memo_components(tuple(MEMOS.values())) - body_source = next( - code for path, code in files if path.endswith("RecursiveBox.jsx") - ) + # The memo mirrors to its source module's combined file (named after the + # module, not the memo), so look it up by that path rather than a per-name + # ``RecursiveBox.jsx``. + segments = memo_paths.module_to_mirrored_segments(definition.source_module) + assert segments is not None + exp_path = compiler_utils.get_memo_module_path(segments) + body_source = next(code for path, code in files if path == exp_path) # ``>= 2``: once for the export, once for the recursive foreach call site. assert body_source.count("RecursiveBox") >= 2 @@ -1442,6 +1567,62 @@ def recursive_box(items: rx.Var[list[int]]) -> rx.Component: assert type(instance).tag == "RecursiveBox" +def test_self_referencing_memo_omits_self_import_from_aggregate(): + """A self-importing memo must not leak its own specifier into the aggregate. + + The mirrored module specifier a memo uses to reference itself must be + stripped from the aggregate import set returned to the frontend-package scan. + """ + + @rx.memo + def recursive_card(items: rx.Var[list[int]]) -> rx.Component: + return rx.box(rx.foreach(items, lambda item: recursive_card(items=items))) + + definition = MEMOS["RecursiveCard"] + segments = memo_paths.module_to_mirrored_segments(definition.source_module) + assert segments is not None + self_specifier = memo_paths.mirrored_library_specifier(segments) + + _, aggregate_imports = compiler.compile_memo_components((definition,)) + assert self_specifier not in aggregate_imports + + +def test_reset_memo_component_classes_recomputes_stale_library(monkeypatch): + """Resetting the class cache re-resolves a memo's library. + + A module that flips to a package across hot-reload compiles keeps its name + but gains an ``index`` segment; without a reset the cached wrapper class + would keep serving the pre-flip specifier. + """ + from reflex_base.components.memo import ( + _get_memo_component_class, + reset_memo_component_classes, + ) + + reset_memo_component_classes() + monkeypatch.setattr( + memo_paths, "module_to_mirrored_segments", lambda module: ("pkgflip",) + ) + flat = _get_memo_component_class("Flip", source_module="pkgflip") + assert flat.library == "$/app_components/pkgflip" + + # The module is now a package: same name, segments gain ``index``. + monkeypatch.setattr( + memo_paths, "module_to_mirrored_segments", lambda module: ("pkgflip", "index") + ) + # Stale until the cache is cleared. + assert ( + _get_memo_component_class("Flip", source_module="pkgflip").library + == "$/app_components/pkgflip" + ) + reset_memo_component_classes() + assert ( + _get_memo_component_class("Flip", source_module="pkgflip").library + == "$/app_components/pkgflip/index" + ) + reset_memo_component_classes() + + def test_self_referencing_var_memo(): """Var-returning memos whose body recursively calls themselves should decorate.""" diff --git a/tests/units/test_app.py b/tests/units/test_app.py index c59bb62875a..cb0341ad638 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -2195,8 +2195,7 @@ def test_app_wrap_compile_theme( # the memoized ``onError`` handler instead of the AppWrap chain. memo_contents = ( web_dir - / constants.Dirs.UTILS - / constants.PageNames.COMPONENTS + / constants.Dirs.APP_COMPONENTS_INTERNAL / f"{error_boundary_tag}{constants.Ext.JSX}" ).read_text() assert "fallbackRender" in memo_contents @@ -2242,7 +2241,9 @@ def _hydrate_fallback_module(web_dir: Path) -> Path: The path to the HydrateFallback memo module. """ return ( - web_dir / constants.Dirs.COMPONENTS_PATH / f"HydrateFallback{constants.Ext.JSX}" + web_dir + / constants.Dirs.APP_COMPONENTS_INTERNAL + / f"HydrateFallback{constants.Ext.JSX}" ) @@ -2265,7 +2266,7 @@ def test_compile_hydrate_fallback_emits_hydrate_fallback( ).read_text() assert ( "export { HydrateFallback as HydrateFallback } " - 'from "$/utils/components/HydrateFallback";' in app_root + 'from "$/app_components/_internal/HydrateFallback";' in app_root ) assert "Hydrating..." in _hydrate_fallback_module(web_dir).read_text() @@ -2298,7 +2299,7 @@ def test_compile_hydrate_fallback_from_config( app_root = ( web_dir / constants.Dirs.PAGES / constants.PageNames.APP_ROOT ).read_text() - assert 'from "$/utils/components/HydrateFallback";' in app_root + assert 'from "$/app_components/_internal/HydrateFallback";' in app_root assert "Fallback from config..." in _hydrate_fallback_module(web_dir).read_text() @@ -2520,14 +2521,44 @@ def test_compile_writes_app_wrap_memo_components( app.add_page(rx.box("Index"), route="/") app._compile() - # Per-memo modules live under .web/utils/components/; each memo wrapper + # Per-memo modules live under .web/app_components/; each memo wrapper # declares its ``library`` as the per-memo file path, so pages import it # directly. - memo_dir = web_dir / constants.Dirs.UTILS / constants.PageNames.COMPONENTS + memo_dir = web_dir / constants.Dirs.APP_COMPONENTS_INTERNAL assert (memo_dir / f"DefaultOverlayComponents{constants.Ext.JSX}").exists() assert (memo_dir / f"MemoizedToastProvider{constants.Ext.JSX}").exists() +def test_compile_dry_run_does_not_prune_or_write_manifest( + compilable_app: tuple[App, Path], + mocker, +) -> None: + """A ``--dry`` compile must not delete memo files or rewrite the manifest.""" + from reflex.compiler import utils as compiler_utils + + conf = rx.Config(app_name="testing") + mocker.patch("reflex_base.config._get_config", return_value=conf) + app, web_dir = compilable_app + app.add_page(rx.box("Index"), route="/") + + # Seed a stale memo file plus a manifest entry that a real compile would + # prune (it is no longer emitted). + stale_rel = f"{constants.Dirs.APP_COMPONENTS_INTERNAL}/Stale{constants.Ext.JSX}" + stale = web_dir / stale_rel + stale.parent.mkdir(parents=True, exist_ok=True) + stale.write_text("// stale", encoding="utf-8") + manifest = web_dir / compiler_utils._MEMO_MANIFEST_FILENAME + manifest.write_text(json.dumps([stale_rel]), encoding="utf-8") + manifest_before = manifest.read_text(encoding="utf-8") + + app._compile(dry_run=True) + + assert stale.exists(), "dry run must not delete stale memo files" + assert manifest.read_text(encoding="utf-8") == manifest_before, ( + "dry run must not rewrite the memo manifest" + ) + + def test_compile_writes_upload_files_provider_app_wrap( compilable_app: tuple[App, Path], mocker, @@ -2589,7 +2620,7 @@ class AppWrapState(State): assert "children" in app_wrap_fn[app_wrap_fn.index("jsx(StateProvider") :] # The extracted memo module carries the state hook instead. - memo_dir = web_dir / constants.Dirs.UTILS / constants.PageNames.COMPONENTS + memo_dir = web_dir / constants.Dirs.APP_COMPONENTS_INTERNAL assert any( "useContext(StateContexts" in memo_file.read_text() for memo_file in memo_dir.glob(f"*{constants.Ext.JSX}") diff --git a/tests/units/utils/test_memo_paths.py b/tests/units/utils/test_memo_paths.py new file mode 100644 index 00000000000..8ba6a4d146f --- /dev/null +++ b/tests/units/utils/test_memo_paths.py @@ -0,0 +1,150 @@ +"""Tests for source-module capture and mirrored-path translation.""" + +from __future__ import annotations + +import importlib.util +from pathlib import Path +from unittest.mock import patch + +from reflex_base.utils import memo_paths + + +def _user_fn(): + """Stand-in for a user-defined function (defined in this test module).""" + + +def test_capture_source_module_returns_user_module(): + captured = memo_paths.capture_source_module(_user_fn) + # Module name depends on how pytest collected this module; either is fine + # so long as it isn't filtered. + assert captured is not None + assert "test_memo_paths" in captured + + +def test_capture_source_module_filters_framework(): + assert memo_paths.capture_source_module(memo_paths.capture_source_module) is None + + +def test_capture_source_module_filters_main(): + fn = type("F", (), {"__module__": "__main__"}) + assert memo_paths.capture_source_module(fn) is None + + +def test_capture_source_module_filters_missing(): + assert memo_paths.capture_source_module(None) is None + fn = type("F", (), {"__module__": ""}) + assert memo_paths.capture_source_module(fn) is None + + +def test_module_to_mirrored_segments_module(): + spec = importlib.util.find_spec("reflex.app") + # Ensure the test runs against a real, non-package module. + assert spec is not None + assert spec.origin + assert not spec.origin.endswith("__init__.py") + segments = memo_paths.module_to_mirrored_segments("reflex.app") + assert segments == ("reflex", "app") + + +def test_module_to_mirrored_segments_package_appends_index(): + # `reflex.components` is a package — its __init__.py origin should + # cause an "index" segment to be appended. + segments = memo_paths.module_to_mirrored_segments("reflex.components") + assert segments == ("reflex", "components", "index") + + +def test_module_to_mirrored_segments_unsafe_segment_rejected(): + assert memo_paths.module_to_mirrored_segments("foo..bar") is None + assert memo_paths.module_to_mirrored_segments("foo./bar") is None + assert memo_paths.module_to_mirrored_segments("..secret") is None + + +def test_module_to_mirrored_segments_rejects_windows_reserved_names(): + # Reserved device names (any case) can't be created as files on Windows, so + # a module with such a segment falls back to the un-mirrored path. + assert memo_paths.module_to_mirrored_segments("myapp.con") is None + assert memo_paths.module_to_mirrored_segments("aux") is None + assert memo_paths.module_to_mirrored_segments("myapp.COM1") is None + assert memo_paths.module_to_mirrored_segments("myapp.lpt9.ui") is None + # A name merely containing a reserved name is fine. + assert memo_paths.module_to_mirrored_segments("myapp.controls") == ( + "myapp", + "controls", + ) + + +def test_segment_is_safe_rejects_trailing_dot_or_space(): + assert memo_paths._segment_is_safe("ok") + assert not memo_paths._segment_is_safe("trailing.") + assert not memo_paths._segment_is_safe("trailing ") + + +def test_is_framework_module_covers_packages_and_prefixes(): + def _fn_in(module: str): + return type("F", (), {"__module__": module}) + + # The package itself and its submodules are framework. + assert memo_paths.capture_source_module(_fn_in("reflex")) is None + assert memo_paths.capture_source_module(_fn_in("reflex.app")) is None + assert memo_paths.capture_source_module(_fn_in("reflex_base")) is None + # The reflex_components_* family matches by prefix. + assert memo_paths.capture_source_module(_fn_in("reflex_components_radix")) is None + # A user module that merely starts with "reflex" but isn't a framework + # package is still mirrored. + assert ( + memo_paths.capture_source_module(_fn_in("reflexion.pages")) == "reflexion.pages" + ) + + +def test_library_for_mirrors_or_falls_back(): + assert ( + memo_paths.library_for("counter_app.ui", "Button") + == "$/app_components/counter_app/ui" + ) + # No source module → per-name internal path. + assert memo_paths.library_for(None, "Button") == "$/app_components/_internal/Button" + # Unsafe module → per-name internal path. + assert ( + memo_paths.library_for("bad..name", "Button") + == "$/app_components/_internal/Button" + ) + + +def test_module_to_mirrored_segments_none(): + assert memo_paths.module_to_mirrored_segments(None) is None + assert memo_paths.module_to_mirrored_segments("") is None + + +def test_mirrored_jsx_path_joins_segments(): + web_dir = Path("/tmp/.web") + path = memo_paths.mirrored_jsx_path(web_dir, ("counter_app", "ui", "buttons")) + assert path == web_dir / "app_components" / "counter_app" / "ui" / "buttons.jsx" + + +def test_mirrored_library_specifier_joins_with_slash(): + spec = memo_paths.mirrored_library_specifier(("counter_app", "ui", "buttons")) + assert spec == "$/app_components/counter_app/ui/buttons" + + +def test_mirrored_paths_live_under_reserved_app_components_dir(): + """User module paths are nested under app_components/, so they can't clobber framework output.""" + assert memo_paths.mirrored_jsx_path(Path(".web"), ("app", "root")) == Path( + ".web/app_components/app/root.jsx" + ) + assert ( + memo_paths.mirrored_library_specifier(("app", "root")) + == "$/app_components/app/root" + ) + + +def test_resolve_user_module_from_frame_skips_framework(): + captured = memo_paths.resolve_user_module_from_frame() + # Must find a user frame — the test module itself qualifies. + assert captured is not None + assert not captured.startswith("reflex") + + +def test_resolve_user_module_from_frame_returns_none_inside_framework_only(): + # Simulate a stack of framework-only frames. + with patch.object(memo_paths, "_is_framework_module", return_value=True): + assert memo_paths.resolve_user_module_from_frame() is None diff --git a/tests/units/utils/test_telemetry_accounting.py b/tests/units/utils/test_telemetry_accounting.py index 7fcd996d055..760c713571a 100644 --- a/tests/units/utils/test_telemetry_accounting.py +++ b/tests/units/utils/test_telemetry_accounting.py @@ -215,13 +215,10 @@ class UserWalkState(rx.State): def test_memo_wrapper_class_records_wrapped_component_type(): """The dynamic memo subclass exposes the user-authored component class.""" - import importlib - + from reflex_base.components.memo import _get_memo_component_class from reflex_components_radix.themes.components.button import Button - memo_module = importlib.import_module("reflex.experimental.memo") - - wrapper_cls = memo_module._get_memo_component_class( + wrapper_cls = _get_memo_component_class( "Button_button_deadbeefcafebabe", Button, )